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

Prevent showing webauthn error for every time visiting /user/settings/security #18385

Merged
merged 2 commits into from
Jan 24, 2022

Conversation

wxiaoguang
Copy link
Contributor

It's quite annoying that the webauthn error is popped up every time when visiting /user/settings/security.

After this fix, the error is only shown after the user clicks the register (add security key) button.

image

@wxiaoguang wxiaoguang added this to the 1.17.0 milestone Jan 24, 2022
@zeripath
Copy link
Contributor

Maybe we should be hiding the register button if detect-webauthn is not detected.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 24, 2022
@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jan 24, 2022

Maybe we should be hiding the register button if detect-webauthn is not detected.

It would be a bigger change, we should find some where to show the error message. Otherwise the users wouldn't know what happened and they would come to ask why I can not use webauthn .....

This PR just works and it seems fine to show the Security Key UI to tell users here we have a nice feature. And I am open, if there is a better PR, feel free to use it to replace this one.

@codecov-commenter

This comment has been minimized.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jan 24, 2022
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jan 24, 2022
@techknowlogick techknowlogick merged commit 9837773 into go-gitea:main Jan 24, 2022
zeripath pushed a commit that referenced this pull request Jan 25, 2022
@wxiaoguang wxiaoguang deleted the fix-webauthn-prompt branch January 25, 2022 02:31
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jan 25, 2022
* giteaoffical/main:
  Place inline diff comment dialogs on split diff in 4th and 8th columns (go-gitea#18403)
  API: Return primary language and repository language stats API URL (go-gitea#18396)
  Update to work with latest VS Code go debugger (go-gitea#18397)
  Fix restore without topic failure (go-gitea#18387)
  [skip ci] Updated translations via Crowdin
  Make WrappedQueues and PersistableChannelUniqueQueues Pausable (go-gitea#18393)
  Fix commit's time (go-gitea#18375)
  Prevent showing webauthn error for every time visiting `/user/settings/security` (go-gitea#18385)
Chianina pushed a commit to Chianina/gitea that referenced this pull request Mar 28, 2022
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants