Conversation
… DS bind methods The @reference to IdentityProviderConfig sat on a Map field, a type Declarative Services cannot inject, so SCR rejected it and bindIdentityProviderConfig was never called: /identityProviders stayed empty and no social auth module was generated. Move the annotation to the bind method, and do the same for AuthenticationService, whose bindIdentityProviderService (which registers the provider listener) was bypassed by field injection too. Also make getIdentityProviderByType return an empty list for a type with no bound provider, and add providers atomically. Fixes OpenIdentityPlatform#225
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #225
Problem
IdentityProviderServicedeclared its@ReferencetoIdentityProviderConfigon aMap<String, List<IdentityProviderConfig>>field. Declarative Services cannot inject that type, so SCR rejected the reference (Field identityProviders ... has unsupported type java.util.Map) andbindIdentityProviderConfig/unbindIdentityProviderConfigwere never called. As a result/identityProvidersalways answered{"providers":[]}, noOPENID_CONNECT/OAUTHauth module was generated, and the social self-service stage received no providers.The annotations were moved onto the fields in 5aa45c8 (the Felix SCR → OSGi DS migration). The same thing happened in
AuthenticationService: its@ReferencetoIdentityProviderServiceis on the field, and because the field type is valid, SCR injects it without logging anything.bindIdentityProviderService, which registers the provider listener, was never called either, so provider changes never reached the authentication filter.Changes
IdentityProviderService: the reference is declared onbindIdentityProviderConfig, with an explicitunbindand the same reference nameidentityProviders. Providers are added atomically (computeIfAbsent+CopyOnWriteArrayList).getIdentityProviderByTypenow returns an empty list for a type with no bound provider. Before this change that path could not be reached; with providers bound it would have thrown an NPE.AuthenticationService: the reference is declared onbindIdentityProviderService, andunbindIdentityProviderServicenow takes the service argument that DS requires. Both methods rebuild the social auth modules. This does nothing before activation, and coversIdentityProviderServicearriving afterAuthenticationServicehas already activated.Generated descriptors:
Tests
IdentityProviderServiceTest: added tests for a lookup by a type with no bound providers, and for unbind (the provider is removed and listeners are notified).AuthenticationServiceTest: added tests that bind registers the listener and that unbind unregisters it and stops injecting providers. The existing tests now callsetConfigafterbind, which is the order SCR uses (bind before activate).bind*directly and so never exercised the SCR wiring. The descriptors above were checked in the built jars.Note: #207 touches the same line in
getIdentityProviderByType, so whichever PR merges second will get a trivial conflict.