[#5] add auth status view of current user#28
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #28 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 13 15 +2
Lines 237 248 +11
Branches 19 20 +1
=========================================
+ Hits 237 248 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| return JsonResponse( | ||
| { | ||
| "auth_status": { | ||
| "authenticated": request.user.is_authenticated, |
There was a problem hiding this comment.
I'm not sure if we wanted to allow anonymous users to access this endpoint but if we don't we can remove this field and return an 401 response instead.
There was a problem hiding this comment.
I am not sure either, why shouldn't we allow them?Can you think of a specific situation?
There was a problem hiding this comment.
I'm not sure if it is mentioned anywhere in the tickets but I recall during discussing this endpoint that it might be expanded on later, with extra information related to authenticated users. You could argue that that information won't be relevant for unauthenticated users and just return the 401 response.
There was a problem hiding this comment.
Yeah, not sure about it..maybe we should ask for an extra opinion from Robin or Anna who have a better overview?
There was a problem hiding this comment.
@robinmolen as you're going to be working with this endpoint, do you have any thoughts/preferences about this?
There was a problem hiding this comment.
Mhh yeah we discussed possible expansions for this endpoint last thursday, but not sure if and when this would happen. I think I'm in favor of returning 401 when the user isn't logged in. Then, when the user is logged in, we return 200 with the MFA verified flag.
401 will make the OF admin-ui a bit simpler, and might be a more generic (Maykin-wide) expected behaviour.
|
I was look into the |
| return JsonResponse( | ||
| { | ||
| "auth_status": { | ||
| "authenticated": request.user.is_authenticated, |
There was a problem hiding this comment.
I am not sure either, why shouldn't we allow them?Can you think of a specific situation?
SonnyBA
force-pushed
the
feature/5-authentication-endpoint
branch
from
5f185d3 to
3a2cb6e
Compare
SonnyBA
force-pushed
the
feature/5-authentication-endpoint
branch
from
3a2cb6e to
c51570f
Compare
sergei-maertens
left a comment
sergei-maertens
left a comment
There was a problem hiding this comment.
Can we also include a default maykin_2fa.api.urls so that all individual projects don't need to invent the endpoint? We can then also hardcode the (relative) URL in the frontend code.
SonnyBA
force-pushed
the
feature/5-authentication-endpoint
branch
2 times, most recently
from
db24ef8 to
59a9ec3
Compare
SonnyBA
force-pushed
the
feature/5-authentication-endpoint
branch
from
59a9ec3 to
cd61fa1
Compare
SonnyBA
force-pushed
the
feature/5-authentication-endpoint
branch
from
cd61fa1 to
4ea96d4
Compare
4 participants
Partly closes open-formulieren/admin-ui#2
Adds a simple view which front-end clients can interact with to retrieve the current user's authentication status.
Required for #5 (which is needed for this admin-ui issue). This new view will be used in OF to provide the front-end with the necessary data.