Skip to content

Run detectWebAuthnSupport only on sign-in page #31676

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.

Sign up for GitHub

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

Merged
techknowlogick merged 2 commits into from
Jul 23, 2024
Merged

Run detectWebAuthnSupport only on sign-in page #31676

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
574d83c
fix: shortcut detectWebAuthnSupport
wolfogre Jul 23, 2024
8240819
Merge branch 'main' into bugfix/detect_web_auth
GiteaBot Jul 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions web_src/js/features/user-auth-webauthn.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import {GET, POST} from '../modules/fetch.ts';
const {appSubUrl} = window.config;

export async function initUserAuthWebAuthn() {
if (!document.querySelector('.user.signin')) {
Copy link
Contributor

@wxiaoguang wxiaoguang Jul 23, 2024

Choose a reason for hiding this comment

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

This check is not right because link_account also uses signin_inner

But I guess some logic has been broken for a while.

Copy link
Member

@silverwind silverwind Jul 23, 2024

Choose a reason for hiding this comment

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

We should probably add a data-page="user-signin" attribute to <body> at some point to clearly indicate the which page we are on instead of relying on these flimsy selectors.

Copy link
Contributor

@wxiaoguang wxiaoguang Jul 24, 2024

Choose a reason for hiding this comment

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

Are you serious? I am talking the logic bug.

Copy link
Member

@silverwind silverwind Jul 24, 2024

Choose a reason for hiding this comment

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

I guess next time put a negative review, because otherwise, every PR runs danger of being merged too hastily.

And yes, my remark was just some idea for the future, but apparently @techknowlogick had read it incorrectly.

return;
}

if (!detectWebAuthnSupport()) {
return;
}
Expand Down