-
Notifications
You must be signed in to change notification settings - Fork 0
Lock down access to authenticator #1
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
Changes from all commits
e0f570e
41f7608
00d3703
dae8b45
80ce6a3
9148bee
0af0500
63e79a0
426add6
d683b09
d332786
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| name: CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| pull_request: | ||
| branches: | ||
| - main | ||
|
|
||
| jobs: | ||
| test: | ||
| name: Run Tests | ||
| runs-on: ubuntu-latest | ||
| if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up JDK 21 | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: '21' | ||
| distribution: 'temurin' | ||
| cache: 'maven' | ||
|
|
||
| # Setup Maven with OAuth token | ||
| - name: Setup Maven with OAuth | ||
| uses: curityio/curity-maven-gh-action@v1 | ||
| with: | ||
| client-secret: ${{ secrets.CURITY_CLI_CLIENT_SECRET }} | ||
|
|
||
| - name: Run tests | ||
| run: mvn verify |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| name: publish.yml | ||
| on: | ||
| push: | ||
| tags: | ||
| - 'v[0-9]+.[0-9]+.[0-9]+' | ||
|
|
||
| jobs: | ||
| package: | ||
| name: Build Package | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up JDK 21 | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| java-version: '21' | ||
| distribution: 'temurin' | ||
| cache: 'maven' | ||
|
|
||
| # Setup Maven with OAuth token | ||
| - name: Setup Maven with OAuth | ||
| id: setup-maven-oauth | ||
| uses: curityio/curity-maven-gh-action@v1 | ||
| with: | ||
| client-secret: ${{ secrets.CURITY_CLI_CLIENT_SECRET }} | ||
|
|
||
| - name: Publish package | ||
| run: mvn deploy ${{ steps.setup-maven-oauth.outputs.maven-deploy-args }} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Changelog | ||
|
|
||
| All notable changes to this project will be documented in this file. | ||
|
|
||
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), | ||
| and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
|
||
| ## [Unreleased] | ||
|
|
||
| ## [1.0.2] - 2025-11-17 | ||
|
|
||
| - Changed plugin implementation type from `access_token` to `access-token`. | ||
|
|
||
| ## [1.0.1] - 2025-11-05 | ||
|
|
||
| ### Security | ||
|
|
||
| - Enforce stricter OAuth client verification - only confidential, authenticated clients are now allowed to perform | ||
| authorization. | ||
|
|
||
| ## [1.0.0] - 2025-11-05 | ||
|
|
||
| ### Added | ||
|
|
||
| - Initial commit. | ||
|
|
||
| ### Technical Details | ||
|
|
||
| - Built with Kotlin. | ||
| - Requires Java 21 or newer | ||
| - Compatible with Curity Identity Server 10.4.2 | ||
| - Uses jose4j for JWT validation | ||
| - Jakarta Validation API 3.0.0 for configuration validation | ||
|
|
||
| [Unreleased]: https://github.com/curityio/access-token-authenticator/compare/v1.0.0...HEAD | ||
|
|
||
| [1.0.0]: https://github.com/curityio/access-token-authenticator/releases/tag/v1.0.0 | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,10 +20,13 @@ import org.jose4j.jwt.consumer.InvalidJwtException | |
| import org.jose4j.jwt.consumer.InvalidJwtSignatureException | ||
| import org.jose4j.jwt.consumer.JwtConsumerBuilder | ||
| import org.slf4j.LoggerFactory | ||
| import se.curity.identityserver.sdk.Nullable | ||
| import se.curity.identityserver.sdk.authentication.AuthenticationResult | ||
| import se.curity.identityserver.sdk.authentication.AuthenticatorRequestHandler | ||
| import se.curity.identityserver.sdk.errors.ErrorCode | ||
| import se.curity.identityserver.sdk.haapi.ProblemContract | ||
| import se.curity.identityserver.sdk.http.MediaType | ||
| import se.curity.identityserver.sdk.oauth.OAuthClient | ||
| import se.curity.identityserver.sdk.service.ExceptionFactory | ||
| import se.curity.identityserver.sdk.web.Request | ||
| import se.curity.identityserver.sdk.web.Response | ||
|
|
@@ -72,9 +75,48 @@ class AccessTokenAuthenticatorRequestHandler( | |
| override fun preProcess( | ||
| request: Request, | ||
| response: Response, | ||
| ): AccessTokenAuthenticatorRequestModel = | ||
| if (request.isGetRequest) AccessTokenAuthenticatorRequestModel.forGet() | ||
| ): AccessTokenAuthenticatorRequestModel { | ||
| enforceHaapiFlow(request, response) | ||
| checkIfOAuthClientIsAllowed(response) | ||
| return if (request.isGetRequest) AccessTokenAuthenticatorRequestModel.forGet() | ||
| else AccessTokenAuthenticatorRequestModel.forPost(request) | ||
| } | ||
|
|
||
| private fun enforceHaapiFlow(request: Request, response: Response) { | ||
| if (request.acceptableMediaTypes != MediaType.HAAPI_JSON.toString()) { | ||
| failAuthentication( | ||
| response, "Request must accept only the Media-Type ${MediaType.HAAPI_JSON} to call this endpoint", | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| private fun checkIfOAuthClientIsAllowed(response: Response) { | ||
| val clientNotAllowed = "OAuth client is not allowed" | ||
|
|
||
| val oauthClient: @Nullable OAuthClient = _config.requestingOAuthClient.client | ||
| ?: failAuthentication( | ||
| response, clientNotAllowed, | ||
| detailedMessage = "The authorization flow was not started by a known OAuth Client, cannot proceed." | ||
| ) | ||
|
|
||
| if (oauthClient.isPublic) { | ||
| failAuthentication( | ||
| response, clientNotAllowed, | ||
| detailedMessage = "The authorization flow was started by a public OAuth Client, cannot proceed." | ||
| ) | ||
| } | ||
|
|
||
| val allowedClients = _config.allowedOauthClientIds | ||
|
|
||
| if (allowedClients.isNotEmpty() && !allowedClients.contains(oauthClient.id)) { | ||
| val allowedClientsText = allowedClients.joinToString(", ") | ||
| failAuthentication( | ||
| response, | ||
| clientNotAllowed, | ||
| detailedMessage = "OAuth client is not allowed, allowed clients are: $allowedClientsText" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, exactly. That's why I put an explict |
||
| ) | ||
| } | ||
| } | ||
|
|
||
| override fun get( | ||
| requestModel: AccessTokenAuthenticatorRequestModel, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,23 +21,24 @@ import se.curity.identityserver.sdk.config.annotation.Description | |
| import se.curity.identityserver.sdk.config.annotation.SizeConstraint | ||
| import se.curity.identityserver.sdk.haapi.RepresentationFunction | ||
| import se.curity.identityserver.sdk.plugin.descriptor.AuthenticatorPluginDescriptor | ||
| import se.curity.identityserver.sdk.service.RequestingOAuthClient | ||
| import se.curity.identityserver.sdk.service.crypto.AsymmetricSignatureVerificationCryptoStore | ||
| import java.util.Optional | ||
|
|
||
| object AccessTokenAuthenticatorConstants { | ||
| const val PLUGIN_TYPE = "access_token" | ||
| const val PLUGIN_TYPE = "access-token" | ||
| const val TEMPLATE_NAME = "authenticate/start" | ||
| } | ||
|
|
||
| /** | ||
| * Plugin configuration object. | ||
| */ | ||
| interface AccessTokenAuthenticatorConfig : Configuration { | ||
| @get:Description("The expected token issuer") | ||
| @get:Description("The expected token issuer.") | ||
| @get:SizeConstraint(min = 2, max = 1024) | ||
| val requiredIssuer: String | ||
|
|
||
| @get:Description("The expected token audience") | ||
| @get:Description("The expected token audience.") | ||
| val requiredAudience: Optional<@SizeConstraint(min = 2, max = 128) String> | ||
|
|
||
| @get:Description("The required scopes, if any.") | ||
|
|
@@ -56,8 +57,20 @@ interface AccessTokenAuthenticatorConfig : Configuration { | |
| @get:SizeConstraint(min = 1, max = 64) | ||
| val subjectClaimName: String | ||
|
|
||
| @get:Description("The IDs of the allowed OAuth clients. If empty, any confidential OAuth client will be allowed.") | ||
| val allowedOauthClientIds: List<@SizeConstraint(min = 1, max = 128) String> | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, Java allows specifying annotations on values of container types :) . Quite handy and we handle this properly in the YANG generator. |
||
|
|
||
| @get:Description("The asymmetric key to use to verify the token signature.") | ||
| val keyVerification: AsymmetricSignatureVerificationCryptoStore | ||
|
|
||
| /** | ||
| * Service to obtain the requesting OAuth client. | ||
| * | ||
| * This authenticator will allow only confidential clients. Consequently, the client must be present. | ||
| * | ||
| * Finally, the OAuth client must be in [allowedOauthClientIds] if that List is not empty. | ||
| */ | ||
| val requestingOAuthClient: RequestingOAuthClient | ||
| } | ||
|
|
||
| /** | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we expect any HAAPI client to send any "complex" Accept header, so I guess it's ok to enforce the header has the exact value we want.