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

feat: common changes for site protection #3878

Merged

Conversation

amk-dev
Copy link
Contributor

@amk-dev amk-dev commented Mar 5, 2024

Fixs HFE-410

Changes

  1. make router beforeEach guard wait for the modules returning promises to resolve before going to the target route
  2. allow guest only access for enter.vue, this is a requirement to prevent getting redirected to login-required page if site protection is enabled, because we use enter for verifying the magic link
  3. add i18n entries for site protection related texts

@amk-dev amk-dev changed the base branch from main to release/2024.3.0 March 5, 2024 20:47
@shipko
Copy link
Contributor

shipko commented Mar 6, 2024

Hi @amk-dev
I agree with you changes. I also want to suggest switch off stacktrace in /graphql method

For example

curl 'https://api.hoppscotch.io/graphql' \
  -H 'authority: api.hoppscotch.io' \
  -H 'accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7' \
  -H 'cache-control: no-cache' \
  --compressed
{
    "errors": [
        {
            "message": "This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight\n",
            "extensions": {
                "code": "BAD_REQUEST",
                "stacktrace": [
                    "BadRequestError: This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight",
                    "",
                    "    at new GraphQLErrorWithCode (/usr/src/app/node_modules/@apollo/server/src/internalErrorClasses.ts:15:5)",
                    "    at new BadRequestError (/usr/src/app/node_modules/@apollo/server/src/internalErrorClasses.ts:116:5)",
                    "    at preventCsrf (/usr/src/app/node_modules/@apollo/server/src/preventCsrf.ts:91:9)",
                    "    at ApolloServer.executeHTTPGraphQLRequest (/usr/src/app/node_modules/@apollo/server/src/ApolloServer.ts:1048:20)",
                    "    at processTicksAndRejections (node:internal/process/task_queues:95:5)"
                ]
            }
        }
    ]
}

@amk-dev
Copy link
Contributor Author

amk-dev commented Mar 6, 2024

Hi @amk-dev I agree with you changes. I also want to suggest switch off stacktrace in /graphql method

For example

curl 'https://api.hoppscotch.io/graphql' \
  -H 'authority: api.hoppscotch.io' \
  -H 'accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7' \
  -H 'cache-control: no-cache' \
  --compressed
{
    "errors": [
        {
            "message": "This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight\n",
            "extensions": {
                "code": "BAD_REQUEST",
                "stacktrace": [
                    "BadRequestError: This operation has been blocked as a potential Cross-Site Request Forgery (CSRF). Please either specify a 'content-type' header (with a type that is not one of application/x-www-form-urlencoded, multipart/form-data, text/plain) or provide a non-empty value for one of the following headers: x-apollo-operation-name, apollo-require-preflight",
                    "",
                    "    at new GraphQLErrorWithCode (/usr/src/app/node_modules/@apollo/server/src/internalErrorClasses.ts:15:5)",
                    "    at new BadRequestError (/usr/src/app/node_modules/@apollo/server/src/internalErrorClasses.ts:116:5)",
                    "    at preventCsrf (/usr/src/app/node_modules/@apollo/server/src/preventCsrf.ts:91:9)",
                    "    at ApolloServer.executeHTTPGraphQLRequest (/usr/src/app/node_modules/@apollo/server/src/ApolloServer.ts:1048:20)",
                    "    at processTicksAndRejections (node:internal/process/task_queues:95:5)"
                ]
            }
        }
    ]
}

Hey, thank you for your suggestion. at the moment we are keeping the stacktraces to help with some frontend logging. since our code is opensource, most issues arising from leaked stack traces are not a concern for us. people have access to our entire codebase.

also if you think this warrants more discussion, you can open another issue or discussion, since this PR is about some frontend changes for a different feature.

@amk-dev amk-dev marked this pull request as ready for review March 6, 2024 13:51
@AndrewBastin AndrewBastin merged commit 6d66d12 into hoppscotch:release/2024.3.0 Mar 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants