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

Consolidate authentication methods #3988

Merged
merged 63 commits into from May 22, 2024
Merged

Consolidate authentication methods #3988

merged 63 commits into from May 22, 2024

Conversation

gguillemas
Copy link
Contributor

@gguillemas gguillemas commented May 6, 2024

Thank you for submitting this pull request! We really appreciate you spending the time to work on these changes.

What is the motivation?

The motivation for this PR is to consolidate all existing authentication methods (except for basic authentication for system users) under the generic concept of access methods, sometimes referred to as simply "access".

The purpose of this change is to implement a breaking change in the SurrealDB interfaces in 2.0 to prepare for future improvements in authentication and authorization, with any new features being a side effect of this change. Further improvements and additional features based on this change will be implemented in separate PRs.

This change aims to:

  • Improve user experience around authentication concepts in SurrealDB, specially around legacy scopes and tokens.
  • Allow more customization when issuing tokens generated by SurrealDB so they can be used by external services.
  • Prepare SurrealDB to incorporate additional access methods and additional capabilities to manage access methods.

The specific motivation for each individual change will be described in the section below.

What does this change do?

Access Methods as a First Class Citizen

Previously, both scopes and tokens were first-class citizens in SurrealDB. With this change, access methods become a single first-class citizen, under which the old functionality from scopes and tokens will reside. This allows SurrealDB to support additional access methods in the future with a consistent API for both users and developers. This will also allow SurrealDB to offer the ability to manage access methods after they have been defined, including auditing the access grants issued with each access method and revoking existing access grants (e.g. tokens), depending on the properties of the access method.

  • Deprecates scopes in favor of record access.

    • Scopes were essentially a way to authenticate against a database record by using arbitrary signup and signin logic defined with SurrealQL. We believe it is more clear and precise to refer to this operation as record access and to refer to users that exist in SurrealDB as database records by "record users" instead of "scope users".
    • Scopes were stored independently, accessible through SurrealQL via the $scope parameter and some entities like tokens could be defined ON SCOPE. Scopes were both a storage location and an authorization level. Now, record access will be stored and behave as any other access method while still providing the same benefits as legacy scopes did. Since scopes were associated with a single database and did not have any children other than tokens (which will be addressed next), the DATABASE storage location will be used instead of SCOPE. Authenticating with a record will provide the authenticated user with the "Record" authorization level, enjoying the same privileges as the old "Scope" authorization level to interact with the database while being limited by PERMISSIONS clauses.
    • Record access will be treated as any other access method, which can only be defined by system owners.
    • Like with scopes, defining a record access method will currently only be possible at the database level.
    • Other than the changes previously mentioned, the current functionality will continue existing under DEFINE ACCESS ... TYPE RECORD, which will keep a similar syntax to the legacy DEFINE SCOPE statement.
  • Deprecates tokens in favor of JWT access.

    • Tokens allowed SurrealDB to trust third-party authentication tokens in order to manage and query SurrealDB. These tokens were specifically JWT, but there are other kinds of tokens that we would like to support in the future. JWT in particular will continue being the main credential used internally by SurrealDB.
    • We have taken the opportunity to be more precise with the syntax (e.g. TOKEN becomes JWT, TYPE becomes ALGORITHM, VALUE becomes KEY) to allow users to clearly associate the SurrealQL syntax with the more widely used concepts. Token verification using JWKS is now implemented separately with the URL clause.
    • Access using JWT for system users can still be defined ON NAMESPACE and ON DATABASE. Access using JWT for record users is now defined as a record access method by using the WITH JWT clause. Example: DEFINE ACCESS example TYPE RECORD ON DATABASE WITH JWT ALGORITHM HS512 KEY 'secret'.
    • All other current functionality will continue existing under DEFINE ACCESS ... TYPE JWT, which will keep a similar syntax to the legacy DEFINE TOKEN statement.

Both legacy statements (i.e. DEFINE SCOPE and DEFINE TOKEN) will continue working to allow for backward compatibility with database exports, existing guides, third-party content... In 2.0, the legacy statements will be parsed and stored in the same way as DEFINE ACCESS, which provides the same functionality as well as additional improvements.

  • Allows SurrealQL to query the access method. Similarly to how the $scope parameter would work, SurrealQL can now query the $access parameter to retrieve information about the access method that was used to authenticate the current system user or record user in SurrealDB. This is specially useful in the case of applications that allow authentication both via record signup/signin with credentials and with a third-party authentication platform. The $token parameter will continue existing, as SurrealDB will continue using JWT to internally manage authentication.

  • Simplifies tokens in SurrealDB. Tokens issued by SurrealDB will contain an ac claim identifying the access method that was used to obtain the token. Likewise, the ac claim indicates the access method that should be used by SurrealDB to verify the token. This makes the sc and tk claims redundant and unnecessary.

Token Issuer Flexibility

When successfully authenticating with record access (or with any other access method introduced in the future), SurrealDB will return a JWT that can be used to keep the authenticated session in SurrealDB. This was already the behavior with legacy scopes. This token could also in theory be trusted by third parties to rely on the authentication performed by SurrealDB for other services, including those without their own authentication. However, keys used by SurrealDB to sign scope tokens were randomly generated and not accessible through any interfaces, preventing this integration for taking place. Expiration for those tokens was also not independently configurable.

This PR introduces the ability to customize how these tokens are generated. Using the WITH JWT ... ISSUER clause, it is now possible to configure token expiration, signing algorithm, singing key, and opens the possibility of adding custom claims to tokens issued by SurrealDB. This addresses #1985 as well as #3500. Customizing the expiration of tokens allows users to configure SurrealDB to issue short-lived tokens that can be used to establish a session during a small time window. After a session is established, users will be able to remain authenticated for the defined session duration.

What is your testing strategy?

Ensure that existing tests continue working. Increase the testing coverage around authentication features.

Is this related to any issues?

Does this change need documentation?

This PR requires documentation for the DEFINE ACCESS statement as well as the "Security" section of the documentation. This documentation will be written in a separate PR and linked here to aid with the review of this PR.

Have you read the Contributing Guidelines?

@gguillemas gguillemas added the topic:security This is related to security label May 6, 2024
@gguillemas gguillemas changed the title [WIP] Consolidate authentication methods Consolidate authentication methods May 13, 2024
@gguillemas gguillemas marked this pull request as ready for review May 13, 2024 13:43
@gguillemas gguillemas requested review from a team and tobiemh as code owners May 13, 2024 13:43
core/src/iam/signin.rs Outdated Show resolved Hide resolved
core/src/iam/signup.rs Outdated Show resolved Hide resolved
core/src/iam/token.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@phughk phughk left a comment

Choose a reason for hiding this comment

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

It's not backwards compatible and it's not feature gated, so as soon as this merges, then we have changed the product. For migrating data and migrating code. It's a really good change! Perhaps in the future I would try branching it out into separate PRs and feed the smaller PRs in one-by-one. Makes it easier to review and reason about.

core/src/err/mod.rs Show resolved Hide resolved
core/src/iam/auth.rs Show resolved Hide resolved
core/src/iam/signin.rs Outdated Show resolved Hide resolved
core/src/iam/signin.rs Show resolved Hide resolved
core/src/key/database/ac.rs Show resolved Hide resolved
core/src/key/namespace/ac.rs Show resolved Hide resolved
core/src/sql/statements/define/mod.rs Show resolved Hide resolved
@gguillemas
Copy link
Contributor Author

gguillemas commented May 14, 2024

It's not backwards compatible and it's not feature gated, so as soon as this merges, then we have changed the product. For migrating data and migrating code. It's a really good change! Perhaps in the future I would try branching it out into separate PRs and feed the smaller PRs in one-by-one. Makes it easier to review and reason about.

Thank you for the review @phughk! I see that I have missed some important context in the description of the PR, which I have since updated. This change is intended to be a breaking change for 2.0, hence why this PR is against main instead of 1.x. It was my understanding that we agreed that those changes may include changes in the public APIs and require datastore migrations.

Regarding splitting the PR, this is already one of the multiple PRs that will be required to implement the change as defined in the RFC. This PR does away with the concept of scopes and tokens (which are heavily interdependent), which requires implementing an alternative (i.e. access methods) to keep the existing functionality. Almost all of the changes in this PR have to do with removing scopes and tokens from everywhere in SurrealDB and replacing them with the new generic alternative. Let me know if you can see any useful ways of splitting this PR further, I am happy to split this into separate PRs if it would be beneficial.

core/src/fnc/mod.rs Show resolved Hide resolved
core/src/iam/entities/resources/level.rs Show resolved Hide resolved
core/src/dbs/session.rs Show resolved Hide resolved
core/src/key/mod.rs Outdated Show resolved Hide resolved
core/src/key/mod.rs Outdated Show resolved Hide resolved
@tobiemh tobiemh added this pull request to the merge queue May 22, 2024
Merged via the queue into main with commit e38b891 May 22, 2024
22 checks passed
@tobiemh tobiemh deleted the gerard/consolidate-auth branch May 22, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:security This is related to security
Projects
None yet
4 participants