Skip to content

#6218 Use extension-specific oauth redirect url#6219

Merged
sosnovsky merged 17 commits into
masterfrom
issue-6218-use-extension-specific-oauth-redirect-url
May 18, 2026
Merged

#6218 Use extension-specific oauth redirect url#6219
sosnovsky merged 17 commits into
masterfrom
issue-6218-use-extension-specific-oauth-redirect-url

Conversation

@martgil

@martgil martgil commented May 7, 2026

Copy link
Copy Markdown
Collaborator

This PR replaces the Google robots.txt redirect URL to extension specific oauth redirect URL.

Closes #6218


Tests (delete all except exactly one):

  • Does not need tests (refactor only, docs or internal changes)

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@martgil martgil requested a review from sosnovsky as a code owner May 7, 2026 02:42

@sosnovsky sosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi @martgil, a lot of tests are failing because local dev extension builds use dynamic extension id. Maybe there is some way to use fixed id for local builds, so we can add this id to the list of allowed redirect URIs for our OAuth app.

@martgil martgil self-assigned this May 7, 2026
@martgil martgil marked this pull request as draft May 7, 2026 08:49
@martgil

martgil commented May 7, 2026

Copy link
Copy Markdown
Collaborator Author

You're correct, Roma. I'll look for some other alternative.

@martgil

martgil commented May 7, 2026

Copy link
Copy Markdown
Collaborator Author

Hello @sosnovsky - Just a quick question. The Mozilla Firefox extension id has also this dynamically generated id, does this means, the extension id for both builds should matches exactly identical? Or does the Mozilla firefox build has to had the static extension id too?

@sosnovsky

Copy link
Copy Markdown
Collaborator

identity.getRedirectURL() in firefox uses id from browser_specific_settings property of manifest.json (https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/identity/getRedirectURL), so probably it shouldn't change between builds and remain static

@sosnovsky

Copy link
Copy Markdown
Collaborator

also firefox uses browser.identity.getRedirectURL() instead of chrome.identity.getRedirectURL()

@martgil

martgil commented May 7, 2026

Copy link
Copy Markdown
Collaborator Author

Got it - thanks!

@martgil

martgil commented May 7, 2026

Copy link
Copy Markdown
Collaborator Author

Hello @sosnovsky - I have managed to make a static extension id from which the current one we have on this PR is nnlimfkjgafpgmilbnaabplbokhpicbo. I have certificate where the key is originated from. It it technically a test file but should I keep it uploaded here somewhere in test/?

@sosnovsky

Copy link
Copy Markdown
Collaborator

is this certificate required for creating local builds? if no, then there is no need to put it in the repository
also your current solution adds key property directly to manifest.json which will probably affect our production builds. I think we should have a separate chrome-consumer-local build and inject key property only for this build type

@martgil

martgil commented May 7, 2026

Copy link
Copy Markdown
Collaborator Author

is this certificate required for creating local builds? if no, then there is no need to put it in the repository

Edit: No, it is not required for creating the local builds. Thanks for your answer.

I think we should have a separate chrome-consumer-local build and inject key property only for this build type

Got it - will do.

@martgil martgil marked this pull request as ready for review May 13, 2026 10:35
@martgil

This comment was marked as outdated.

Comment thread extension/manifest.json Outdated
@martgil martgil requested a review from sosnovsky May 14, 2026 08:48

@sosnovsky sosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

well done 👍

Comment thread extension/js/common/api/authentication/google/google-oauth.ts
Comment thread extension/js/common/api/authentication/google/google-oauth.ts
@martgil martgil requested a review from sosnovsky May 15, 2026 07:47
@martgil

martgil commented May 15, 2026

Copy link
Copy Markdown
Collaborator Author

Hi @sosnovsky - I have manually tested it on Firefox and it works well now. Then tests for the chrome build have passed too. Feel free to re-check at your convenience.

@sosnovsky sosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

works great now, well done 👍

Comment thread extension/js/common/api/authentication/generic/oauth.ts Outdated
@martgil martgil requested a review from sosnovsky May 18, 2026 04:35

@sosnovsky sosnovsky left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

all good now, thanks!

@sosnovsky sosnovsky merged commit 8f25298 into master May 18, 2026
12 checks passed
@sosnovsky sosnovsky deleted the issue-6218-use-extension-specific-oauth-redirect-url branch May 18, 2026 09:20
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.

Use extension-specific OAuth redirect URL instead of Google robots.txt

2 participants