Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add interface IterableRelyingPartyRegistrationRepository or similar #15027

Closed
OrangeDog opened this issue May 8, 2024 · 1 comment
Closed
Assignees
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement
Milestone

Comments

@OrangeDog
Copy link
Contributor

Expected Behavior

public class InMemoryRelyingPartyRegistrationRepository
        implements IterableRelyingPartyRegistrationRepository {

Current Behavior

public class InMemoryRelyingPartyRegistrationRepository
        implements RelyingPartyRegistrationRepository, Iterable<RelyingPartyRegistration> {

Context
A mess develops when you start adding custom implementations that are also iterable, and/or writing e.g. custom login pages that need to iterate through all registrations. Especially if you're trying to compose multiple repositories because they need different behaviour,

You either have to resort to unchecked casts, or unpleasant generics like <T extends RelyingPartyRegistrationRepository & Iterable<RelyingPartyRegistration>>.

I assume it's too late to make RelyingPartyRegistrationRepository extend Iterable<RelyingPartyRegistration> or to add a method that returns a Collection<RelyingPartyRegistration>?

@OrangeDog OrangeDog added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels May 8, 2024
@OrangeDog
Copy link
Contributor Author

OrangeDog commented May 8, 2024

More context: I'm composing multiple repositories because I'm implementing metadata refresh, and it's considerably easier if a RefreshableRelyingPartyRegistrationRepository is only responsible for a single URI, as each EntitiesDescriptor (or indeed HTTP response) can define its own caching and validity.

Although, as I've been typing this, I think it might be easier if I instead write a RelyingPartyRegistrationRepository adaptor for MetadataResolver. Then I don't have to reimplement everything that OpenSAML already does. You might consider adding that yourselves.

Edit: except you've made that difficult because OpenSamlMetadataRelyingPartyRegistrationConverter is not public.

Edit 2: also ChainingMetadataResolver does not implement IterableMetadataSource, but you cannot do anything about that.

@sjohnr sjohnr added the in: saml2 An issue in SAML2 modules label May 9, 2024
@jzheaux jzheaux added this to the 6.4.x milestone May 31, 2024
@jzheaux jzheaux removed the status: waiting-for-triage An issue we've not yet triaged label May 31, 2024
jzheaux added a commit to jzheaux/spring-security that referenced this issue Jul 2, 2024
@jzheaux jzheaux closed this as completed in 1e29003 Jul 2, 2024
@jzheaux jzheaux modified the milestones: 6.4.x, 6.4.0-M1 Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement
Projects
Archived in project
Development

No branches or pull requests

3 participants