Skip to content

Remove old google sign-in implementation.#3449

Merged
sea-snake merged 9 commits intomainfrom
sea-snake/remove-old-google-sign-in-implementation
Oct 28, 2025
Merged

Remove old google sign-in implementation.#3449
sea-snake merged 9 commits intomainfrom
sea-snake/remove-old-google-sign-in-implementation

Conversation

@sea-snake
Copy link
Copy Markdown
Contributor

@sea-snake sea-snake commented Oct 27, 2025

Remove old Google sign-in implementation since this has been superseded by the generic OpenID implementation.

Changes

  • Remove openid_google config and related state.
  • Remove openid/google.rs.
  • Remove setup_google from openid.rs.
  • Remove openid_google and related types from did file.
  • Remove all sign in with Google code from II 1.0 (had to be removed since it relied on above types).
  • Remove (un)link account astro views.
  • Remove all Google specific OpenID implementation code from II 2.0.
  • Remove OPENID_AUTHENTICATION feature flag since it's no longer used.
  • Fix integration/config/rate_limit.rs should verify register_rate_limit instead of related_origins.
  • Fix integration/config/archive_config.rs should verify archive_config instead of related_origins.

Tests

Verified that II 1.0 and 2.0 work as expected, additionally e2e tests should pass.


🔴 Some screens were removed

@sea-snake sea-snake requested a review from lmuntaner October 27, 2025 21:11
Copy link
Copy Markdown
Contributor

@lmuntaner lmuntaner left a comment

Choose a reason for hiding this comment

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

It must have been a nice PR to make!!

Minor comments

Comment thread src/frontend/src/lib/utils/openID.ts Outdated
openid_google: Some(Some(OpenIdGoogleConfig {
client_id: "https://example.com".into(),
})),
related_origins: Some(vec!["https://example.com".into()]),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This integration tests checks that config property A doesn't change when another property e.g. B is changed. Since B (openid_google) isn't available anymore, I switched to another random property C (related_origins).

openid_google: Some(Some(OpenIdGoogleConfig {
client_id: "https://example.com".into(),
})),
enable_dapps_explorer: Some(true),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why is this necessary?

Copy link
Copy Markdown
Contributor Author

@sea-snake sea-snake Oct 28, 2025

Choose a reason for hiding this comment

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

See above comment, couldn't use C here since it's the same property as A in this particular test file.

openid_google: Some(Some(OpenIdConfig {
client_id: "https://example.com".into(),
})),
related_origins: Some(vec!["https://example.com".into()]),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See my other comment.

@sea-snake sea-snake requested a review from lmuntaner October 28, 2025 09:24
@sea-snake sea-snake added this pull request to the merge queue Oct 28, 2025
Merged via the queue into main with commit 0fce586 Oct 28, 2025
76 checks passed
@sea-snake sea-snake deleted the sea-snake/remove-old-google-sign-in-implementation branch October 28, 2025 09:48
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.

2 participants