Skip to content

Single-key Key Selector #7055

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

Merged
merged 1 commit into from
Jun 28, 2019
Merged

Single-key Key Selector #7055

merged 1 commit into from
Jun 28, 2019

Conversation

jzheaux
Copy link
Contributor

@jzheaux jzheaux commented Jun 28, 2019

Fixes: gh-7049

@jzheaux jzheaux self-assigned this Jun 28, 2019
@jzheaux jzheaux requested a review from jgrandja June 28, 2019 17:18
@jzheaux jzheaux added in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue type: improvement labels Jun 28, 2019
@jzheaux jzheaux added this to the 5.2.0.RC1 milestone Jun 28, 2019
Copy link
Member

@rwinch rwinch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This looks good. Go ahead and merge once you have thought about if SingleKeyJWSKeySelector should be submitted to Nimbus

* @author Josh Cummings
* @since 5.2
*/
final class SingleKeyJWSKeySelector<C extends SecurityContext> implements JWSKeySelector<C> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider submitting this to Nimbus?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking on this. Yes, I'm planning on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@jgrandja jgrandja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jzheaux. I left a couple comments.

JWSKeySelector<SecurityContext> jwsKeySelector =
new JWSVerificationKeySelector<>(this.jwsAlgorithm, jwkSource);
new SingleKeyJWSKeySelector<>(this.jwsAlgorithm, this.secretKey);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should have changed since it's the SecretKey builder and the issue raised is for PublicKey.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I've raised #7056 so that this gets articulated.

private final List<Key> keySet;
private final JWSAlgorithm expectedJwsAlgorithm;

public SingleKeyJWSKeySelector(JWSAlgorithm expectedJwsAlgorithm, Key key) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove the public modifier since the class is package-private

@jzheaux jzheaux merged commit ce79ef2 into spring-projects:master Jun 28, 2019
@jzheaux jzheaux deleted the gh-7049 branch June 28, 2019 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: oauth2 An issue in OAuth2 modules (oauth2-core, oauth2-client, oauth2-resource-server, oauth2-jose) status: duplicate A duplicate of another issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NimbusJwtDecoder.withPublicKey() does not work with JWT that contains kid
3 participants