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

@auth: Validate token's account scope #563

Merged
merged 1 commit into from
Oct 23, 2024
Merged

Conversation

arturpimentel
Copy link
Contributor

What is the purpose of this pull request?

Check whether the token's indicated account is the request's context account.

What problem is this solving?

Until now, the @auth logic allows for cross-account requests as long as the target account contains the user that is identified by the token. This is not usually a problem since the token has to be a valid VTEX ID token and the user mail has to have the necessary access scope in the target account in order for this to work. However, it is a conceptual error (and a security flaw) to allow for cross-account requests in this manner unless the resource is explicitly allowed by a ReBAc rule, which is not the case here (GraphQL fields are not ReBAc-sensitive in VTEX IO).

How should this be manually tested?

  • Create/Use a user that exists in vtexgame1, but in a different account;
  • Take an admin token from that user in that account;
    Use that token to make the following request:
curl --location 'https://testauth--vtexgame1.myvtex.com/_v/private/graphql/v1' \
--header "Cookie: ${token}" \
--header 'Content-Type: application/json' \
--data-raw '{"query":"{ \n    availableApps() @context(provider: \"[email protected]\") {    id    vendor    version    name    slug    icon    title    featured    categories    registry  }}","variables":{}}'

You should get an 200 response but with an error body like the one below:

{
    "data": {
        "availableApps": null
    },
    "errors": [
        {
            "message": "Could not find user specified by VtexIdclientAutCookie.",
            "locations": [
                {
                    "line": 2,
                    "column": 3
                }
            ],
            "path": [
                "vtex_appsgraphql_3_17_4_availableApps"
            ],
            "extensions": {
                "code": "UNAUTHENTICATED",
                "exception": {
                    "message": "Could not find user specified by VtexIdclientAutCookie.",
                    "name": "AuthenticationError",
                    "extensions": {
                        "code": "UNAUTHENTICATED"
                    },
                    "sensitive": {
                        "stack": "AuthenticationError: Could not find user specified by VtexIdclientAutCookie.\n    at ..."
}

Screenshots or example usage

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Requires change to documentation, which has been updated accordingly.

Copy link

@silvadenisaraujo silvadenisaraujo left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

Check whether the token's indicated account is the request's context
account. Until now, the @auth logic allows for cross-account requests
as long as the target account cotains the user that is identified by the
token.
@iagoaraujo iagoaraujo merged commit 36f8bdf into master Oct 23, 2024
2 of 3 checks passed
@iagoaraujo iagoaraujo deleted the fix/directives/auth branch October 23, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants