Lock down access to authenticator #1
Conversation
Must be a HAAPI flow. Must be a confidential client. Optionally must be one of the allowed clients.
renatoathaydes
force-pushed
the
lock-down-access-to-authenticator
branch
from
431ab37 to
41f7608
Compare
| } | ||
|
|
||
| private fun enforceHaapiFlow(request: Request, response: Response) { | ||
| if (request.acceptableMediaTypes != MediaType.HAAPI_JSON.toString()) { |
There was a problem hiding this comment.
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, | ||
| clientNotAllowed, | ||
| detailedMessage = "OAuth client is not allowed, allowed clients are: $allowedClientsText" |
There was a problem hiding this comment.
Will allowedClientsText only be visible if detailed message are disabled in the profile? We should not reveal client IDs for other clients in prod.
There was a problem hiding this comment.
Yes, exactly. That's why I put an explict detailedMessage parameter there.
Also, this is tested in the main repo.
| 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.
This @SizeConstraint applies to the String and not to the List, right? I always have doubts on this.
There was a problem hiding this comment.
Correct, Java allows specifying annotations on values of container types :) . Quite handy and we handle this properly in the YANG generator.
| val keyVerification: AsymmetricSignatureVerificationCryptoStore | ||
|
|
||
| /** | ||
| * This authenticator will allow only confidential clients. |
There was a problem hiding this comment.
Detail: perhaps
Service to obtain the requesting client.
That client must be in [allowedOauthClientIds], if this list is not empty.
There was a problem hiding this comment.
You're right, I should've mentioned it. Adding it.
renatoathaydes
force-pushed
the
lock-down-access-to-authenticator
branch
from
eba1c75 to
0af0500
Compare
renatoathaydes
force-pushed
the
lock-down-access-to-authenticator
branch
from
43d1db3 to
63e79a0
Compare
| 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:: |
There was a problem hiding this comment.
I think you can use a separate header for this:
Warning
~~~~~~
This plugin...
You can use different level of header if you don't want it to be H2.
|
|
||
| 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. |
There was a problem hiding this comment.
Maybe:
You can use this plugin with the Hypermedia Authentication API (`HAAPI`_). If a user tries to run an authentication flow with this authenticator from a browser, they will see an error.
Only confidential clients with the HAAPI capability are able to use this authenticator in an authentication flow.
| * ``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 |
There was a problem hiding this comment.
I would rewrite slightly:
OAuth clients added to the ``allowed-oauth-client-ids`` list must be confidential clients. Public clients will be rejected when trying to use this authenticator, even if they are on the list. This is to ensure that only a limited set of OAuth clients, ones that the authorization server trusts, will have the power to obtain sensitive tokens on behalf of end users.
This limitation applies only to the client that runs an authentication flow with this authenticator. The access token used as the input to this authenticator can be obtained by any OAuth client, even a public one.
As @pmhsfelix mentioned, we were not doing enough validations about the OAuth client performing the HAAPI flow.
This PR locks down which HAAPI client is allowed to be used, and forbids completely non-HAAPI clients, by:
haapicapability.allowed-oauth-client-idsexplicitly in case the customer desires to lock it down even more. It's reasonable in some cases only a particular server will perform authentication on behalf of end users.