-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Do not show passkey on http sites #33820
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both http localhost (visible) and something else on http (no error, no webauthn) are working for me, looks much better now without the error ❤️
* Backport #33348 * Backport #33820 --------- Co-authored-by: yp05327 <[email protected]>
* giteaofficial/main: Move notifywatch to service layer (go-gitea#33825) [skip ci] Updated translations via Crowdin Only keep popular licenses (go-gitea#33832) Removing unwanted ui container (go-gitea#33833) Full-file syntax highlighting for diff pages (go-gitea#33766) Improve theme display (go-gitea#30671) Decouple context from repository related structs (go-gitea#33823) Improve log format (go-gitea#33814) Decouple diff stats query from actual diffing (go-gitea#33810) Add global lock for migrations to make upgrade more safe with multiple replications (go-gitea#33706) Do not show passkey on http sites (go-gitea#33820)
Fix go-gitea#33615 (cherry picked from commit b8c2afd)
hideElem(elSignInPasskeyBtn); | ||
return; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just because I didn't know it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Care for a PR? I'm not sure I undertand the full context of this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to open a PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said, I don't fully understand. Do you think window.location.protocol === 'http:' && !window.isSecureContext
is the right condition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea, I think the old approach is quite trivial and won't cause any real problem. The time could be spent on something more important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just replace the whole by
if (!window.isSecureContext) {
hideElem(elSignInPasskeyBtn);
return;
}
I have tested this and this seem to be equivalent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that seems reasonable, I will open a PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix #33615