malparty / keywords

Codding challenge
0 stars 0 forks source link

Handling missing userId in controllers #29

Closed olivierobert closed 3 years ago

olivierobert commented 3 years ago

Since a user needs to be authenticated to perform any action, there are checks in each controller method:

image

The issues:

malparty commented 3 years ago

Nice catch! yes, it totally make sense. I initially send a "NotFound()" as the "Authorized" attribute is already handling 401 (and redirection to the login page) for non-authenticated users. So as part of the object retrieval logic it seemed more like a notfound. But assuming we isolate this, indeed a 401 would makes more sense. For this situation, as the Authorized attribute already manage the user authentication (and confirm us the user is well loggedin), I would suggest a 403 error.

olivierobert commented 3 years ago

Sure. 403 makes sense at the user is presumed authenticated past this point. The PR does resolve the issue.