Skip to content
This repository has been archived by the owner on Feb 27, 2020. It is now read-only.

Add support for multiple IdentityProviders #55

Open
stevesaliman opened this issue Aug 8, 2018 · 4 comments · May be fixed by #62
Open

Add support for multiple IdentityProviders #55

stevesaliman opened this issue Aug 8, 2018 · 4 comments · May be fixed by #62

Comments

@stevesaliman
Copy link

I have the following use case I'm trying to implement:

We have a Service Provider, and we want our clients to be able to use it using their Identity Provider. This means we'll need to have more than one Identity Provider. I have an implementation that works, but I'm not sure if the approach is what you'd have in mind to solve the problem, which is why I haven't done a pull request for it. The full code is in the multi-idp branch of my fork, but the basics work like this:

First, I made the SAMLConfigurer's identityProvider a list. Then I made the SAMLConfigurer's identityProvider() method create a new instance of IdentityProvider and add it to that list.

Next, I made some modifications to how the metadata gets created so it works with multiple IDPs.

Finally, I added the following method to SAMLConfigurer to delegate some of the configuration to the class doing the security configuration:

    public SAMLConfigurer delegateConfig(Function<SAMLConfigurer, SAMLConfigurer> delegate) {
        return delegate.apply(this);
    }

It looks a little weird, but I couldn't create an IdentityProvider outside the SAMLConfigurer, and the delegate lets me use the dsl in helper methods like the one I use to iterate over our supported IDPS, doing the following for each one:

	samlConfigurer
		.identityProvider()
			.metadataFilePath("file://" + f.getAbsolutePath())
			.metadataTrustCheckEnabled(false);

I can then do this in my WebSecurityConfigurerAdapter implementation:

	.http(saml())
		.apply(saml())
			.serviceProvider()
				.keyStore()
					.storeFilePath(samlProperties.getKeystore())
					.password(samlProperties.getKeystorePassword())
					.keyname(samlProperties.getDefaultKey())
					.keyPassword(samlProperties.getDefaultKeyPassword())
					.and()
				.protocol("http")
				.hostname("localhost:8080")
				.basePath("/")
				.and()
			.delegateConfig(this::idpHelper)

What do you think?

@hardikns
Copy link

hardikns commented Jan 7, 2019

@stevesaliman, Thanks for this. Do you have any sample code somewhere. Also, what do you plan to put in idpHelper? Is it just idp-configuration or something else?

@stevesaliman
Copy link
Author

I took a stab at implementing this in my fork of the project. I didn't submit a pull request because I have fixes for several issues in it, and I didn't have the best tests for it, but you're welcome to pull in and use what you like from it.

my idpHelper is intended to add multiple identityProvider instances to the configuration, but it could be used to add any valid element to the provided SAMLConfigurer instance. If it helps, my current implementation of idpHelper, makes an identity provider for each file it finds in a directory, and it looks like this:

	private SAMLConfigurer idpHelper(SAMLConfigurer samlConfigurer) {
		File metadataDir = new File(maxProperties.getDataDirectory(), "IdpMetadata");
		// If the metadata directory doesn't exist, we won't be able to do a
		// SAML login, but that may be fine, so instead of reporting an error,
		// we'll make the directory in case we need to put something there
		// later.
		if ( !metadataDir.exists() ) {
			try {
				FileUtil.makeDirectory(metadataDir);
			} catch (IOException e) {
				// We don't want to send a method that throws checked exceptions
				// to the Saml configurer.
				LOG.error("Unable to create metadata directory " + metadataDir, e);
				throw new RuntimeException(e);
			}
		}

		File[] files = metadataDir.listFiles((dir, name) -> name.endsWith(".xml"));
		if ( files != null && files.length > 0 ) {
			for ( File f : files ) {
				// Our metadata files are stored on disk, so it is less important
				// to verify signatures.  For now, skip it to buy us time to
				// figure out key management.
				samlConfigurer
					.identityProvider()
						.metadataFilePath("file://" + f.getAbsolutePath())
						.metadataTrustCheckEnabled(false);
			}
		}
		return samlConfigurer;
	}

@rwinch
Copy link
Collaborator

rwinch commented Jan 15, 2019

@stevesaliman Thanks for the pointer. We'd be interested in any PRs that you might have. This project has hit an unofficial maintenance mode as we rework our SAML story.

@stevesaliman
Copy link
Author

The code base I have contains fixes for a couple of issues I was having, and I don't really have a good separation of them. Would it be acceptable to do a single PR with all of my changes?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
3 participants