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

OIDC introspection endpoint #295

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

byewokko
Copy link
Collaborator

@byewokko byewokko commented Oct 3, 2023

closes #283

Changes

OAuth 2.0 Token Introspection

  • Found at POST /openidconnect/token/introspect.
  • The POST payload must be query-encoded and must include token with the token value and client_id.
  • The endpoint must not be exposed to the public network since there is no client authentication implemented (except for the client_id query parameter requirement).

Example

Request

POST /openidconnect/token/introspect
client_id=example-app&token=HQHNO7fC8iFB0MuQJvohVIYaxC-KGIEMg6PyIBQUYmIf9jPbxM2oceM-A1U3v3hd

Response

{
	"active": true,
	"token_type": "access_token",
	"client_id": "example-app",
	"exp": 1697023308,
	"iat": 1697019708,
	"sub": "mongodb:ext:abcd....",
	"aud": "example-app",
	"iss": "https://auth.local.loc/seacat-auth/api",
	"username": "pepik"
}

@byewokko byewokko added the enhancement New feature or request label Oct 3, 2023
@byewokko byewokko self-assigned this Oct 3, 2023
@byewokko byewokko marked this pull request as draft October 3, 2023 15:07
@byewokko byewokko marked this pull request as ready for review October 11, 2023 10:51
@byewokko byewokko requested a review from ateska October 11, 2023 10:53
Copy link
Contributor

@ateska ateska left a comment

Choose a reason for hiding this comment

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

Ok from my side.

@filipmelik
Copy link

filipmelik commented Oct 12, 2023

The POST payload must be query-encoded and must include token with the token value and client_id.

Why the client_id? As far as I can see, it is a non standard field regarding the linked OAuth2.0 specs. If I understand correctly, this serves as a obscure security measure. Specs clearly states that

To prevent token scanning attacks, the endpoint MUST also require
   some form of authorization to access this endpoint, such as client
   authentication as described in OAuth 2.0 [[RFC6749](https://datatracker.ietf.org/doc/html/rfc6749)] or a separate
   OAuth 2.0 access token such as the bearer token described in OAuth
   2.0 Bearer Token Usage [[RFC6750](https://datatracker.ietf.org/doc/html/rfc6750)].

which is clearly broken here and instead you rely on this one sentence:

The endpoint must not be exposed to the public network since there is no client authentication implemented (except for the client_id query parameter requirement).

IMHO this is insufficient and breaking the spec.

@ateska
Copy link
Contributor

ateska commented Oct 12, 2023

AFAIK @filipmelik - if you want to expose it to public network, the NGINX introspection needs to be put in front of the SeaCat API and that will give you required access authorization.

It also means that the client_id will be handled by that introspection point.

@byewokko

@@ -80,12 +80,12 @@ async def introspect(self, request):
client_id = params.get("client_id")

Choose a reason for hiding this comment

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

As commented in the MR Overview, I would ditch this alltogether in favor of i.e. OAuth2.0 Client Basic Authz.

Copy link
Collaborator Author

@byewokko byewokko Oct 13, 2023

Choose a reason for hiding this comment

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

Including the client_id and client_secret in the POST body is actually a form of OAuth2.0 Client Basic Authentication. Although admittedly I haven't included the client_secret parameter, since we only support public clients, which do not need any secret (they can have one, but it adds little to no security since anyone can extract the secret).

Anyway, we will yet rethink the authentication.

Choose a reason for hiding this comment

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

Clarified on call

seacatauth/openidconnect/handler/introspect.py Outdated Show resolved Hide resolved
seacatauth/openidconnect/handler/introspect.py Outdated Show resolved Hide resolved
@filipmelik
Copy link

AFAIK @filipmelik - if you want to expose it to public network, the NGINX introspection needs to be put in front of the SeaCat API and that will give you required access authorization.

It also means that the client_id will be handled by that introspection point.

not sure I understand TBH.

I (as an external party) see Seacat as an Authorization provider. I do not know (and also don't wanna know) the inner details of how seacat works, it it somehow uses nginx or whatever as a system component and if it have some private API endpoints and how they work. That's why I do not know what do you mean by NGINX introspection. 😊

I see it just as an OAuth2.0 Authorization provider and expect all the endpoints the OAuth2.0 spec supports. And AFAIK the OAuth2.0 endpoints should be by nature "publicly" available. That's why I do not see the point of having introspect endpoint "hidden". 🤔 Can you elaborate pls?

seacatauth/openidconnect/handler/introspect.py Outdated Show resolved Hide resolved
representing the meta information surrounding the token, including whether this token is currently active.

To protect this endpoint with authorization (as required by RFC7662), use NGINX reverse proxy
with auth_request to an NGINX introspection endpoint, for example:

Choose a reason for hiding this comment

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

I guess this need more documentation. So as a seacat user:

  1. what endpoints should I call?
  2. where do I get the credentials?
  3. are those credentials different per each oauth client?

Choose a reason for hiding this comment

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

Clarified on call

@byewokko byewokko marked this pull request as draft October 13, 2023 11:03
@ateska
Copy link
Contributor

ateska commented Oct 16, 2023

@filipmelik - in the nutshell, SeaCat Auth executes authorisation by "introspection" which is delegated to NGINX (that's NGINX introspection).

This is a high-level schema:

Frontend <---> NGINX <---> Backend
                 |   \
                 |    \
          SeaCat Auth - (private API)

How this apply to OIDC introspection endpoint?

In this case, the endpoint is exposed on the SeaCat Auth private API (hence Robin's remarks above); then the exposure to "frontends" happens thru "NGINX introspection" and you get the authorization.
In other words, the SeaCat Auth private API is "just another backend".
It could be easily two microservices.

Ok?

@@ -120,7 +120,8 @@ async def introspect(self, request):
if "preferred_username" in user_info:
response_data["username"] = user_info["preferred_username"]

# TODO: Verify that the token is client_id is included in the aud claim
# TODO: Verify that the requesting client is part of the token's intended audience
# (i.e. that their client_id is included in the aud claim).

Choose a reason for hiding this comment

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

This will break my usecase in mobile apps. Can we have a call about thiz plz?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i don't intend to implement this check within this merge request (or anytime soon, unless necessary). it would also require implementing a way for the client to influence the content of the audience claim somehow, possibly by scope.

i added the todo as a reminder that the authorization has some limitations and can be improved. the authorization server should not disclose id tokens to just any client. it is mostly fine in environments where there are few clients manually registered by an admin, but not in large environments with open client registration.

Choose a reason for hiding this comment

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

Clarified on call

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement OAuth token introspection endpoint
3 participants