Implement Member SSO (extracted from #617)#637
Conversation
All changes made in #617 that are strictly related to login (no OIDC)
|
gl hf |
| def _send_otp_email(member: Member, otp: str) -> None: | ||
| full_code = f"F-{otp}" | ||
| print(f"Send F-code: {full_code}") | ||
| send_fcode_mail(member, full_code, f"hffps://{otp}", "linky") |
There was a problem hiding this comment.
hffps? Hyper F-klub F-code (p)For Stregsystemet?
|
60 commits in #617? Damn, that one would take months to review xD It looks nice though |
Ty, it should be manageable |
Mast3rwaf1z
left a comment
Mast3rwaf1z
left a comment
There was a problem hiding this comment.
Feature looks really nice, there's some general code quality things
I think the pages are very pretty - but i think you should spend some time to make it consistent with the rest of the stregsystem? maybe refactor some of the css into something more generic
There was a problem hiding this comment.
please put in a seperate commit ✨
There was a problem hiding this comment.
this is too pretty for the stregsystem
| {% block title %}Log ind{% endblock %} | ||
|
|
||
| {% block head %} | ||
| <style> |
There was a problem hiding this comment.
not strictly related to this pr, but i think it would be nice to have shared css?
|
|
||
| <div class="stage-dots"><span class="active"></span><span></span></div> | ||
| <h1>Kære Fember</h1> | ||
| <p class="subtitle">Indtast din stregbruger for at fortsætte.</p> |
There was a problem hiding this comment.
is stregbruger the common name for a user for the users?
| } | ||
|
|
||
| def get(self, request): | ||
| ctx = self._base_context(request) |
There was a problem hiding this comment.
i kinda hate the ctx pattern
|
|
||
| return member.paired_user | ||
|
|
||
| def get_user(self, user_id): |
There was a problem hiding this comment.
what type is user_id? could be str, int, bytes or whatever?
There was a problem hiding this comment.
function can return None, and the return statement doesn't make it clear what it returns
I'm not too familiar with django but i feel like objects.get(pk=user_id) could return vector[User] or User
|
|
||
| class MemberOTPRequest(models.Model): | ||
| member = models.ForeignKey(Member, on_delete=models.CASCADE) | ||
| code = models.CharField(max_length=6) |
There was a problem hiding this comment.
might we end up supporting configurable length later?
| fresh = MemberOTPRequest.objects.get(member=self.member, is_valid=True) | ||
| self.assertNotEqual(fresh.code, self.otp) | ||
|
|
||
| # def test_max_attempts_shows_resend_message(self): |
There was a problem hiding this comment.
any reason this is commented out? please write a todo or something
| ) | ||
|
|
||
|
|
||
| def send_fcode_mail(member, fcode, link, redirect_url): |
|
I will get it looked at 😎 |
2 participants
All changes made in #617 that are strictly related to login (no OIDC)
This adds a login system for members which can be used to authenticate members outside trusted network or in public settings,
Adds
/ffo/login-endpoint