-
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 4 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,61 @@ | ||
| 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 | ||
|
|
||
| package: | ||
| name: Build Package | ||
| runs-on: ubuntu-latest | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' | ||
|
|
||
| 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: Build package | ||
| run: mvn package | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| # 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.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 |
|---|---|---|
|
|
@@ -9,7 +9,12 @@ AccessTokenAuthenticator Authenticator Plug-in | |
|
|
||
| A custom Authenticator plugin for the Curity Identity Server. | ||
|
|
||
| This plugin allows users to authenticate using HAAPI by first obtaining an access token via other means. | ||
| .. warning:: | ||
| This plugin cannot be used by users to authenticate directly from a browser. | ||
|
||
| Only authentication via `HAAPI`_ is allowed and the OAuth client initiating authorization MUST be | ||
| a confidential client with the HAAPI capability. | ||
|
|
||
| This plugin allows users to authenticate using `HAAPI`_ by first obtaining an access token via other means. | ||
|
|
||
| That allows a form of token exchange where the end user may be prompted to consent to upscoping, for example. | ||
|
|
||
|
|
@@ -20,15 +25,19 @@ The following configuration settings are available: | |
| * ``required-scopes`` - required token scopes. Optional. | ||
| * ``required-purpose`` - required token ``purpose``. Default: ``access_token``. If set to a blank string, this will be ignored. | ||
| * ``subject-claim-name`` - the name of the subject claim. Default: ``sub``. | ||
| * ``allowed-oauth-client-ids`` - the allowed OAuth clients. If empty, any confidential HAAPI client will be allowed. | ||
| * ``key-verification/id`` - ID of an existing token signature verification key. | ||
|
|
||
| .. note:: | ||
| Even if an OAuth client is allowed by the ``allowed-oauth-client-ids`` setting, it will NOT be allowed to perform authorization | ||
|
||
| unless it is a confidential, authenticated client. This is to ensure that only a limited set of OAuth clients | ||
| that can be trusted will have the power to obtain sensitive tokens on behalf of end users. | ||
| This applies only to the OAuth client performing the authorization flow within which this authenticator | ||
| will be called, not to the OAuth client that obtained the presented access token. | ||
|
|
||
| .. image:: docs/images/access_token_config.png | ||
| :alt: Access Token Authenticator Configuration | ||
|
|
||
| .. note:: | ||
| This plugin should not be used by users to authenticate using a browser because it is a bad security practice to expose | ||
| access tokens directly to end users. Use `HAAPI`_ instead. | ||
|
|
||
| Building the Plugin | ||
| ~~~~~~~~~~~~~~~~~~~ | ||
|
|
||
|
|
||
| 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()) { | ||
|
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. 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. |
||
| 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,6 +21,7 @@ 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 | ||
|
|
||
|
|
@@ -33,11 +34,11 @@ object AccessTokenAuthenticatorConstants { | |
| * 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,18 @@ 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 | ||
|
|
||
| /** | ||
| * This authenticator will allow only confidential clients. | ||
|
||
| * | ||
| * The OAuth client ID must also be allowed by [allowedOauthClientIds]. | ||
| */ | ||
| 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 think you can use a separate header for this:
You can use different level of header if you don't want it to be H2.