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

New oauth2 [Do not merge] #854

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

New oauth2 [Do not merge] #854

wants to merge 18 commits into from

Conversation

hsinn0
Copy link
Contributor

@hsinn0 hsinn0 commented Feb 20, 2025

PR from new-oauth2 feature branch. Not to be merged until the feature completes later. Creating the PR now so as to keep the state of build/test and difference from main in check.

hsinn0 and others added 10 commits February 11, 2025 11:46
- Made the `oauth2ResourceServer` code block compilable.
- Added a couple of new oauth2-specific spring-security dependencies that are reuired in runtime in `:components:auth` sub-project. They are likely to be neeeded in other sub-projects, too.
- In `AuthConfigurationTest`, `infoCanBeAccessedWithoutAuthentication()` and `healthCanBeAccessWithoutAuthentication()` pass. Other cases that have something to do with actual auth either fail or took too long for me to able to see the result.

Co-authored-by: Joe Eltgroth <[email protected]>
- Rewrote the class in Java, replacing the use of the old oauth2 lib with new lib.
- Made all test cases except 4 mTLS-related ones pass in `AuthConfigurationTest`.
- Have not tried to run the production code, i.e. run the credhub server application yet.

Co-authored-by: Joe Eltgroth <[email protected]>
- Made temporary changes in `UserContextFactoryTest.kt` to avoid compile errors.
- Reordered imports in `AuthConfiguration.java` to satisfy `checkstyle`.
- Resolved ktlint errors in `UserContextFactory.kt` and `UserContextFactoryTest.kt`.
- Corrected wrong filter orders.

Co-authored-by: Joe Eltgroth <[email protected]>
- It was failing if `security.oauth2.resource.jwt.key_value' property was not set. We need that propperty for unit-test and backward compatibility.
- Modified `jwtDecoder` bean to honor 'jwt.key_value' property if it exists. Otherwise, use the JWK Set URI.
- With this change, credhub server app starts without any noticeable issues. Basic credhub-cli commands such as `login` and `find` also work.
- Needs more testing against different ednpoints of the running credhub server.

Co-authored-by: Joe Eltgroth <[email protected]>
- Modified the method signature to declare specific Exceptions instead of the base checked exception class.
hsinn0 and others added 8 commits February 19, 2025 16:53
- Had a space at the beginning of the vaiue.
- Was not an issue with old oauth2 lib.
- But `spring-security-oauth2-resource-server` considers it as malformed.
unauthenticated (401) with detailed information about the token'd deficiency.

This seems consistent with the new Spring Security api.
Also the published REST api does not specify error responses.

We may need to rethink this, however, if end-to-end testing shows
other systems are relying on the detailed response.
- Spying `oAuth2IssuerService!!.getIssuer()` does not work with modified `AuthConfiguration` as `jwtDecoder` bean is created at the startup.
- Instead, to manipulate the issuerURI, just replace `jwtDecoder`'s `JwtValidator` to a desired one.
- Also changed `AuthConfiguration.jwtDecoder()` to return `NimbusJwtDecoder` so we don't have to downcast it in the test code.
* Replaced Kotlin AuthWithoutOAuthConfiguration with Java
* New Java class uses Spring Security 5 features and
  eliminates obsolete Spring Security OAuth
- Remove custom classes no longer necessary after new
  Spring Security configurations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants