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

Added a TOTP secret-key input field in the automatic login section #145

Merged
merged 3 commits into from
Feb 9, 2024

Conversation

f10d0
Copy link
Contributor

@f10d0 f10d0 commented Feb 8, 2024

Pull Request

Description

This is regarding the 2FA token input field.
As Noxdor seems to be occupied, but I thought this is a neat thing to have especially now that 2FA seems to become mandatory.
I added an input field located right beneath the user and password login credentials.
As of right now this is only for the TOTP secret key.
The key needs to be entered in base32 encoding.

image

References

Referenced Issue:
#138

Type of change

  • Bug fix (non-breaking change which fixes a bug)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that might cause existing functionality to not work as expected)

Further info

  • This change requires a documentation update
  • I updated the documentation accordingly, if required
  • I commented my code where its useful

Testing

  • Tested my changes on Firefox
  • Tested my changes on Chromium-Based-Browsers
  • Successfully run npm run test locally

@f10d0 f10d0 requested a review from a team as a code owner February 8, 2024 21:33
@f10d0 f10d0 requested a review from OliEfr February 8, 2024 21:33
@OliEfr
Copy link
Member

OliEfr commented Feb 9, 2024

Hi.
Looks good! Two considerations:

  • If the users only saves the TOTP key it still says 'Nicht gespeichert' at the top and it is not possible to click on 'Daten löschen' at the bottom. I am not entirely sure what the intuitive behavior should be, but I assume as soon as either the selma-login or the totp key is saved it should say 'Aktuell gespeichert' and possible to click on 'Daten löschen'.
    image
  • Sorry for asking again, but the TOTP-Key is generated in the authenticator app, right? Can we include a short sentence on how the useres can retrieve this token and maybe suggest a 2FA-App for this to make the feature accessible? (I have no TU Dresden Login anymore so I cannot do the process myself.)

@f10d0
Copy link
Contributor Author

f10d0 commented Feb 9, 2024

I fixed your first point exactly how you described, now if either information is stored you should be able to delete it correctly.

Now the TOTP-Secret-Key itself is generated by the website (the ZIH portal), and then essentially just copied to the authenticator app by scanning the qr-code the website gives. I wrote a markdown file explaining how to extract this key for a few apps and linked it in the automatic login section of the plugin. As I included some screenshots i thought it would be beneficial to have this as a markdown file here on github so the plugin is not unnecessarily bloated with images. Please let me know what you think

@OliEfr
Copy link
Member

OliEfr commented Feb 9, 2024

Thank you. It looks very good to me.
Many more users might use that feature now.

@C0ntroller can you have a quick second look at the code implementation as some user data handling is involved?

docs/2FA.md Show resolved Hide resolved
@OliEfr OliEfr merged commit 2d8abcc into TUfast-TUD:develop Feb 9, 2024
1 check passed
@OliEfr
Copy link
Member

OliEfr commented Feb 9, 2024

Will be released with v8.1.0.1 next week.

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.

3 participants