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 cognito authentication #340

Closed
wants to merge 5 commits into from

Conversation

eddumelendez
Copy link
Contributor

No description provided.

Copy link
Contributor

@maciejwalkowiak maciejwalkowiak left a comment

Choose a reason for hiding this comment

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

Please add reference docs and sample app.

* @since 2.3
*/
@ConfigurationProperties(prefix = "spring.cloud.aws.security.cognito")
public class CognitoAuthenticationProperties {
Copy link
Contributor

Choose a reason for hiding this comment

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

nullability annotations are missing. Each package should have package-info.java marking everything as @NonNull by default, and all nullable fields, method parameters and return values should be marked with @Nullable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH right now I don't see value adding those annotations in configuration properties. I agree if those are added in public apis such as *template

Copy link
Contributor

Choose a reason for hiding this comment

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

If we don't add them, everything is @NonNull by default. While not such a big deal for Java, when used with Kotlin non-null fields are seen as non-nullable types which can cause very unexpected NPEs in Kotlin Land

@maciejwalkowiak maciejwalkowiak added this to the 3.0.0 M2 milestone May 6, 2022
@github-actions github-actions bot added the type: documentation Documentation or Samples related issue label May 16, 2022
@sonarcloud
Copy link

sonarcloud bot commented May 16, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@maciejwalkowiak
Copy link
Contributor

As far as I can see, we are missing taking into account spring.cloud.aws.endpoint and a sample. Or it's just not possible to support it?

@eddumelendez
Copy link
Contributor Author

we are not using the cognito sdk here so the use of spring.cloud.aws.endpoint is not something possible. The only way I see is being explicit with the use of registry-url and issuer-url. About the sample. yes, it is pending.

@eddumelendez
Copy link
Contributor Author

Closing this due to can be easily autoconfigured using spring boot features

spring.security.oauth2.resourceserver.jwt.issuer-uri=http://127.0.0.1:4566/us-east-1_f865f8979c4d4361b6af703db533dbb4
spring.security.oauth2.resourceserver.jwt.jwk-set-uri=http://127.0.0.1:4566/us-east-1_f865f8979c4d4361b6af703db533dbb4/.well-known/jwks.json

See more here

@maciejwalkowiak
Copy link
Contributor

@eddumelendez so you're saying that beans that we used to create before NimbusJwtDecoder and DelegatingOAuth2TokenValidator are not needed anymore?

@maciejwalkowiak maciejwalkowiak removed this from the 3.0.0 M2 milestone Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation Documentation or Samples related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants