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

Make SuperTokens an OAuth and Open ID provider #582

Open
52 of 87 tasks
Tracked by #1054
rishabhpoddar opened this issue Mar 16, 2023 · 7 comments
Open
52 of 87 tasks
Tracked by #1054

Make SuperTokens an OAuth and Open ID provider #582

rishabhpoddar opened this issue Mar 16, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@rishabhpoddar
Copy link
Member

rishabhpoddar commented Mar 16, 2023

Opened this in favour of: https://github.com/supertokens/for-zenhub/issues/108

We want to support:

Google doc discussion here:

Test case TODOs

  • Test case 1..

TODO

  • Flow diagrams
  • CDI repo changes
  • add an api in the core that takes refresh token + client id to return info of the tokens table that is associated with the refresh token
  • add an api in the core that takes auth code + client id to return the info of the tokens table that is associated with the auth code.
  • add another column in tokens table like “client_secret_required_for_refresh” which will be true if NOT using PKCE.
  • the scopes set in the createTokens endpoint should be a subset of the ones set in the authorization code if using the auth code flow to create tokens.
  • What happens if you have taken a lock on the tokens table based on refresh token hash and client id, and then in parallel, the client id is removed from the db:
    • Will the lock fail? But what if the lock is taken first?
    • Will the removing of client id wait for all the locks to be released.
    • Will both go ahead, but the commit of of lock query fail and it will retry?
  • Add a new field called grantType to OAuth2TokenInfo. The grantType field should be an enum that includes the following values: AUTH_CODE, REFRESH_TOKEN, AUTH_CODE_PKCE, and CLIENT_CREDENTIALS.
  • Add query_string field of type TEXT to auth_code and access_token tables
  • Update the cdi spec and flow diagram of create auth_code, create access_token, verify access_token to include query_string param in request as well as response.
  • Write down the list of all response status codes across all APIs
  • buildAccessToken and buildIdToken recipe functions should also return a useDynamicSigningKey arg (which will be true by default).
  • The core token POST API should accept a necessary boolean called useDynamicSigningKey . This API should check if the lifetime of the access token is greater than the dynamic signing key interval and if useDynamicSigningKey is true, then we throw a 400 error. Similar checks should also be done for session recipe and jwt recipe in the core.
  • We need to build it in a way that people can use us just as an oauth server, without actually using us as an identity server (similar to authlete or ory hydra)

TODOs with hydra:

  • Ory multi tenancy:
    • One URL per user
      • I don’t think this is possible by default, I think they are running multiple instances even in their own network
    • One db per user (not a hard requirement)
      • Nope: Hydra only has a single DB connection
  • Database table prefix
    • I haven’t seen a table prefix option, but we can specify DB names
  • M2M:
  • How would JWKS endpoint work in the context of sharing on hydra instance across many users - would they all have the same keys?
  • Cleanup of jwt and openid recipe in backend sdk level?
    • While we are likely not going to be able to remove them (at least without seriously rethinking and most likely dropping support for some usecases)
    • The openId recipe will need to be somewhat reworked to interact with the new Hydra
  • checkout hydra github issues to see if there is anything that's really broken
    • In general it seems OK, nothing I've seen seems to be majorly concerning.
  • check if hydra is stateless and if we can add many instances of it behind a load balancer, connected to one db
  • when it comes to things like lifetime of access, refresh, id tokens on a per client basis.
  • are the scopes definable as per what the user likes? For example, can the scope be a random string entirely per client or does it have to be from a global pool of allowed scopes?
    • Not entirely random strings (it has to match the standard) but no global pool of scopes
  • How do they do things like access token blacklisting?
  • can users specify a custom lifetime for auth code per client?
    • Yes
  • Do they support PKCE flow?
    • Yes
  • In general, we want to think about all the kinds of customisations that we would provide if we had built it in house, and check how it’s possible to do them with our Hydra.
    • They provide a much more broad implementation of the standard than we could feasibly achieve in the next year I think
    • Token/userinfo/auth flow customizations are entirely in our control on the BE sdk level
  • Since the access tokens will be signed by the same private key across apps, then devs must check the audience before using the access token.. We will be resigning the access tokens returned by hydra in the core. By default, we use static signing keys to create the jwt, but this can be core config.
  • Deep links as redirect links. Also allow http.
  • Reduce the number of redirections when navigating to oauth provider, and also when navigating to client app
  • remove ory from the oauth code token.
  • Should we make oauth access token into jwt?
  • create a new function in the backend sdk like verifyOAuthAccessToken, which is like getSession, but it only works for oauth access token.
  • create a new function in the backend sdk like verifyOAuthIdToken
  • Add third party like behaviour for OAuthClient recipe and enforce that it can only be used for supertokens oauth provider. It will not create a new user id mapping (like what happens in third party recipe), and instead reuse the supertokens user id. It should check that the supertokens user id is already existing in the tenant we are trying to login with, and if it doesn't we throw an error. This adds a restriction that all client apps use just one supertokens core db, and that is ok. We will have to come up with a good enough error message here.
  • Checkout how auth0 has adding of custom claims.
  • Check if creating a new session marks the user as active. If not, we may need a separate core API for OAuth2Client Sign in.
  • Check out single sign out and logoutRequests in more detail
  • How do we expect client apps to handle errors like if they pass incorrect scope? In this case, the oauth provider will redirect back to the client app like this http://localhost.com:3005/auth/callback/ory?error=invalid_scope&error_description=The+requested+scope+is+invalid%2C+unknown%2C+or+malformed.+The+OAuth+2.0+Client+is+not+allowed+to+request+scope+%27profilee%27.&state=%EF%BF%BD%EF%BF%BD%EF%BF%BD%EF%BF%BD%EF%BF%BD%EF%BF%BDv%EF%BF%BD%EF%BF%BD%EF%BF%BD%EF%BF%BD%EF%BF%BD
    • We expect them to handle it by displaying the appropriate error message, as it is standard. In most cases, these should pop-up during development or when setting up a new client.
  • hydra allows authorization url endpoint to be called without a redirect_uri. Should we also allow it?
    • In most use-cases will be calling with redirect_uri, but I see no strong reason to disallow it.
    • We have to test that the validation are done as-per the standard.
  • Allow clients to specify tenantId when redirecting to the oauth provider.
    • it can be specified using the tenant_id query param in the auth url
      • this makes the oauth flow only accept session that belong to the right tenant - otherwise it forces a re-auth
      • it sets the tenantId queryparam when redirecting to the auth page
    • the frontend now looks at the tenantId queryparam and only then defaulting to use public
  • Use a very specific version of hydra (v2.2.0) in saas, and not just the latest one.
  • What to do of existing open id recipe in backend sdk?
  • One important thing to discuss is how can users configure right the permissions / scopes / claims for clients via api calls as opposed to only via override.
  • Update loginChallengeInfoGET
    • add clientId
    • make URIs optional
    • ensure FE and BE types are consistent
  • Rename recipe to OAuth2Provider
  • Double check tenant id loading/interactions on the FE
  • Implement rendering logoUri on the FE
  • Discuss about OIDC discovery enpoint and JWKS endpoint
  • Docs:
    • Need to explain that allowed origins in cors need to change to add all expected clients
  • https://supertokens.slack.com/archives/C0662C34JFN/p1721854466624639
  • We've decided not to implement single sign out, but provide token a revocation endpoint
  • We've decided to auto initialize the OAuth2 recipe (make the openid recipe initialize OAuth2, but take care that we use the instance/config the user initializes if they init it themselves)
  • We've decided to "re-sign" the access tokens issued by hydra to avoid multitenancy issues
  • TODO: come up with some kind of convenience function to handle deciding which CORS origins are allowed globally
  • no regex should be allowed for redirectUris! (OAuth2.1)
  • only allow client_secret_basic and none for tokenEndpointAuthMethod (core validation)
  • Client type changes/notes in node:
    • We've decided to make clientId and clientSecret not customizable (generated in the Core)
    • skipConsent will always be set to true in the core (removed from the interface)
    • accessTokenStrategy will always be set to jwt in the core (remove from the interface)
    • subjectType will always be set to public (remove from the interface)
    • Remove not required props: tokenEndpointAuthSigningAlg sectorIdentifierUri , all logout related ones, requestUris, requestObjectSigningAlg ,jwks, jwksUri, userinfoSignedResponseAlg, registrationAccessToken, registrationClientUri , owner, contacts, jwtBearerGrantAccessTokenLifespan
  • Recipe Interface changes for OAuth2Client
    • Remove authorisationUrlGET from the API interface and associated inputs from config because we expect users to use a library and to avoid the extra API call.
    • Rename rawUserInfoFromProvider to rawUserInfo
    • Rename redirectURIOnProviderDashboard to redirectURI
    • Remove session from the input of signInPOST
    • Rename /oauth2client/signin path to oauth/client/signin
  • Create PRs for the examples app
    • Create PR to the supertokens-auth-react repo for web examples apps
    • Create PR to the supertokens-react-native repo for react-native example apps
  • For the docs:
    • The main reason to use OAuth2Client recipe on the backend is that users will be able to use our sessions. Otherwise they can skip using that, and just use standard oauth 2 sdk on the frontend using PKCE.
    • When creating clients for web apps, we will tell people to create clients in a way that doesn't require token exchange endpoint to have auth - so that they can use frontend libs that do pkce.
    • Specify that state (at least 8 characters) long is required in the authorization url. As per the RFC, it is recommended but not Required. However, Hydra makes it a requirement.
    • When using validateOAuth2AccessToken, we need to tell people that if it's a 401, then they also have to return WWW-Authenticate: Bearer error="invalid_token", error_description="The access token expired" header.
    • We need to also say is that for aws gateway, we need to add WWW-Authenticate header
    • When navigating to web view in mobile, we need to tell users to add the login params in authorizationurl
@rishabhpoddar rishabhpoddar added the enhancement New feature or request label Mar 16, 2023
@ykdojo
Copy link

ykdojo commented May 9, 2023

Thank you for sending this to me. Neither of the documents is publicly visible, though.

@rishabhpoddar
Copy link
Member Author

@ykdojo those docs are internal at the moment. You can follow this issue to be notified when we release this feature.

@rishabhpoddar
Copy link
Member Author

Cronjobs

TODO

@rishabhpoddar
Copy link
Member Author

rishabhpoddar commented Jun 6, 2023

New core configs

  • oauth2_auth_code_expiry_after_ms
    • default value is 5601000 (5 mins)
    • validations: value should be <= 10 mins && >= 10 seconds
  • oauth2_access_token_expiry_after_ms
    • default value is 60601000 (1 hour)
    • validations: value should be <= 30 days && >= 1 minute
  • oauth2_resfresh_token_expiry_after_ms
    • default value is 246060*1000 (24 hours)
    • validations: value should be <= 90 days && >= 1 hour

@rishabhpoddar
Copy link
Member Author

Screenshot 2024-04-30 at 12 23 47

  • Only the supertokens core talks to hydra.
  • The supertokens core has a list of client ids that are specific to that app (cud, app), which it uses as a filter before querying hydra.
  • The core must always have a strict subset of the clients that are there in ory, and from the point of view of our backend sdk, those clients are all that exists.
  • The frontend and backend sdk will have to be built out entirely anyway.

@tamassoltesz
Copy link
Contributor

tamassoltesz commented Aug 2, 2024

Hydra /admin/clients endpoint behaviours:
GET
/admin/clients/{clientId}

  1. existing ClientID: returns 200 and the client json in body
  2. not existing clientID: returns 404, and the following json:
{
    ""error"": ""Unable to locate the resource"",
    ""error_description"": """"
}

Expected core response:

  1. in case of found client return 200 and the client info in json body.
  2. in case of not found client: return 200 with something like {status: OAUTH2_CLIENT_NOT_FOUND_ERROR}

DELETE
/admin/clients/{clientId}

  1. in case of existing client id: 204 with empty body
  2. in case of not existing client id: 404 - previously mentioned "Unable to locates the resource" json body.

Expected core response:

  1. in case of successful delete: 200 - {status: OK}
  2. in case of not found client: 200 - {status: OAUTH2_CLIENT_NOT_FOUND_ERROR}

POST
/admin/clients with json body

  1. valid json body in every aspect: 201 - created client as json body
  2. trying to create a client with already existing client id: 409 and the following json body:
"{
    ""error"": ""Unable to insert or update resource because a resource with that value exists already"",
    ""error_description"": """"
}"
  1. invalid json body: 400 - and a json in the {error:..., error_description:...} format.
    Invalid json body here means that some field contained a not allowed value or some field was missing from the input json.

Expected core behaviour:

  1. succesful create, core returns: 200 - {status: OK, client: {created_client_json}}
  2. already existing id: core silently generates a new id and retries until success or other type of error happens
  3. invalid json: core returns: BadRequestException(error - errorDescription)

PATCH
/admin/clients/{clientId} with jsonBody

  1. empty json as body: 500, jsonBody: {error: "error", error_description: "The error is unrecognizable"}
  2. no body: see 1.
  3. missing required fields from body: see 1.
  4. valid json body: 200, updated client info
  5. valid json format, operation with invalid value: see 1.
  6. invalid {clientId}: 404 jsonBody {error: "Unable to locate resource", error_description: ""}
  7. invalid content as value (for example * as allowed_cors_origins: 400, {error: errorMsg, error_description: errorDescriptionMsg}
  8. Using the wrong method (for example replace for an empty list instead of add: see 1.

expected core behaviour:

  1. in case of 500 response from hydra: return 500, "Internal Error"
  2. in case of 400 response from hydra: return BadRequestException(error - errorDescription)
  3. in case of 404 response from hydra: return 200, {status: OAUTH2_CLIENT_NOT_FOUND_ERROR}
  4. in case of 200 response from hydra: return 200, {status: OK, client: <updated_client_from_hydra>}

@tamassoltesz
Copy link
Contributor

tamassoltesz commented Aug 2, 2024

Hydra /oauth2/auth endpoint behaviour:
GET
Only GET is supported. Input params are passed as query params.
Response is sent with headers. Location and Set-Cookies are the interesting ones for us.

  1. valid auth query: 302 - Location header set to: {configured hydra login uri}/login..., Set-Cookie header with ory_hydra_login_csrf_....=...
  2. missing required param: 302 - Location header set to {hydra_instance}/{error_fallback} with query params of error and error_description
  3. invalid value in required param: 303 - Location header set to {clients_redirect_uri} with error and error_description query params
  4. not existing client: 302 - Location header set to {hydra_instance}/{error_fallback} with query params of error and error_description

Expected core behaviour:

  1. replace the {configured hydra login uri} address with the string {apiDomain}. Response in the format of {status: OK, redirectTo: {apiDomain}..., cookies: [set-cookies]}
  2. return 200 - {status: OAUTH2_AUTH_ERROR, error: <error_from_hydra>, errorDescription: <error_description_from_hydra>}
  3. return 200 - Response in the format of {status: OK, redirectTo: returned_redirectTo, cookies: [set-cookies]}
  4. like case 2: 200 - {status: OAUTH2_AUTH_ERROR, error: <error_from_hydra>, errorDescription: <error_description_from_hydra>}

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

No branches or pull requests

3 participants