1370220 - handle 404s from /api/users/current better

Status: RESOLVED FIXED

(Release Engineering Graveyard :: Applications: Balrog (frontend), enhancement, P3)

Description

[[e-mail] (:bhearsum)]

Current, if a user has no permissions nor roles, any request to /api/users/current will generate a 404. This is correct (the user doesn't exist in the db), but in order to support this kind of "read only" user, we shouldn't an error for it.

Comment 1

[Vikas Prasad Mahato [:vikas_mahato]]

Hi,
I am getting started with open source and would like to work on this bug.Can you assign it to me?

Comment 2

[[e-mail] (:bhearsum)]

(In reply to Vikas Prasad Mahato from comment #1)
> Hi,
> I am getting started with open source and would like to work on this bug.Can
> you assign it to me?

Absolutely!

Comment 3

[Vikas Prasad Mahato [:vikas_mahato]]

I have set up my development environment as per the instructions given on http://mozilla-balrog.readthedocs.io/en/latest/contribute.html
Please guide me on how to proceed further with this bug

Comment 4

[[e-mail] (:bhearsum)]

(In reply to Vikas Prasad Mahato from comment #3)
> I have set up my development environment as per the instructions given on
> http://mozilla-balrog.readthedocs.io/en/latest/contribute.html
> Please guide me on how to proceed further with this bug

Unfortunately, this bug is very difficult to reproduce locally, but what we're trying to do is stop sending 404s when the current user has no Permissions or Roles, and queries /api/users/current. This endpoint is implemented in https://github.com/mozilla/balrog/blob/28af89fcc56c7ed88669ef4c33df53161ef2ba76/auslib/web/admin/views/permissions.py#L32, so that's the code you'll need to touch. You should also adjust and add (as necessary) test cases for this new behaviour in https://github.com/mozilla/balrog/blob/28af89fcc56c7ed88669ef4c33df53161ef2ba76/auslib/test/admin/views/test_permissions.py#L20.

It's important that we continue to return 404s when a _specific_ user is requested, eg: /api/users/[e-mail] -- we should only avoid sending them for /api/users/current.

Comment 5

[Vikas Prasad Mahato [:vikas_mahato]]

(In reply to Ben Hearsum (:bhearsum) from comment #4)
> (In reply to Vikas Prasad Mahato from comment #3)
> > I have set up my development environment as per the instructions given on
> > http://mozilla-balrog.readthedocs.io/en/latest/contribute.html
> > Please guide me on how to proceed further with this bug
> 
> Unfortunately, this bug is very difficult to reproduce locally, but what
> we're trying to do is stop sending 404s when the current user has no
> Permissions or Roles, and queries /api/users/current. This endpoint is
> implemented in
> https://github.com/mozilla/balrog/blob/
> 28af89fcc56c7ed88669ef4c33df53161ef2ba76/auslib/web/admin/views/permissions.
> py#L32, so that's the code you'll need to touch. You should also adjust and
> add (as necessary) test cases for this new behaviour in
> https://github.com/mozilla/balrog/blob/
> 28af89fcc56c7ed88669ef4c33df53161ef2ba76/auslib/test/admin/views/
> test_permissions.py#L20.
> 
> It's important that we continue to return 404s when a _specific_ user is
> requested, eg: /api/users/[e-mail] -- we should only avoid sending them
> for /api/users/current.


What should be expected when /api/users/current is requested and the current user has no Permissions or Roles

Comment 6

[[e-mail] (:bhearsum)]

(In reply to Vikas Prasad Mahato from comment #5)
> (In reply to Ben Hearsum (:bhearsum) from comment #4)
> > (In reply to Vikas Prasad Mahato from comment #3)
> > > I have set up my development environment as per the instructions given on
> > > http://mozilla-balrog.readthedocs.io/en/latest/contribute.html
> > > Please guide me on how to proceed further with this bug
> > 
> > Unfortunately, this bug is very difficult to reproduce locally, but what
> > we're trying to do is stop sending 404s when the current user has no
> > Permissions or Roles, and queries /api/users/current. This endpoint is
> > implemented in
> > https://github.com/mozilla/balrog/blob/
> > 28af89fcc56c7ed88669ef4c33df53161ef2ba76/auslib/web/admin/views/permissions.
> > py#L32, so that's the code you'll need to touch. You should also adjust and
> > add (as necessary) test cases for this new behaviour in
> > https://github.com/mozilla/balrog/blob/
> > 28af89fcc56c7ed88669ef4c33df53161ef2ba76/auslib/test/admin/views/
> > test_permissions.py#L20.
> > 
> > It's important that we continue to return 404s when a _specific_ user is
> > requested, eg: /api/users/[e-mail] -- we should only avoid sending them
> > for /api/users/current.
> 
> 
> What should be expected when /api/users/current is requested and the current
> user has no Permissions or Roles

A 200 should be returned with the following json in the body:
{
  "permissions": {}, 
  "roles": {}, 
  "username": "[e-mail]"
}

Comment 7

[Vikas Prasad Mahato [:vikas_mahato]]

Can you please see this and let me know whether this would work since I can't reproduce the bug locally.

Comment 8

[Vikas Prasad Mahato [:vikas_mahato]]

(In reply to Vikas Prasad Mahato from comment #7)
> Can you please see this and let me know whether this would work since I
> can't reproduce the bug locally.

https://github.com/mozilla/balrog/compare/master...vikasmahato:bug_1370220

Comment 9

[[e-mail] (:bhearsum)]

(In reply to Vikas Prasad Mahato from comment #8)
> (In reply to Vikas Prasad Mahato from comment #7)
> > Can you please see this and let me know whether this would work since I
> > can't reproduce the bug locally.
> 
> https://github.com/mozilla/balrog/compare/master...vikasmahato:bug_1370220

Can you open a pull request please? I can leave feedback on that.

Comment 10

[Vikas Prasad Mahato [:vikas_mahato]]

(In reply to Ben Hearsum (:bhearsum) from comment #9)
> (In reply to Vikas Prasad Mahato from comment #8)
> > (In reply to Vikas Prasad Mahato from comment #7)
> > > Can you please see this and let me know whether this would work since I
> > > can't reproduce the bug locally.
> > 
> > https://github.com/mozilla/balrog/compare/master...vikasmahato:bug_1370220
> 
> Can you open a pull request please? I can leave feedback on that.

Done

Comment 12

[Vikas Prasad Mahato [:vikas_mahato]]

I have requested a review on my pull request and waiting for a reply. Can you please comment on it so that I can continue with the bug.

Comment 13

[[e-mail] (:bhearsum)]

(In reply to Vikas Prasad Mahato from comment #12)
> I have requested a review on my pull request and waiting for a reply. Can
> you please comment on it so that I can continue with the bug.

I think I replied to your latest question https://github.com/mozilla/balrog/pull/371#discussion_r134329828? Please let me know if I've missed something.

Comment 14

[Vikas Prasad Mahato [:vikas_mahato]]

I have updated the pull request. Can you review it?
https://github.com/mozilla/balrog/pull/371

Comment 15

[[github robot]]

Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/fc1892df1196f732bf7b6ca5bc4b4bd6d56d5c3d
bug 1370220: handle 404s from /api/users/current better (#371). r=bhearsum

Comment 16

[[e-mail] (:bhearsum)]

This is now in production, nice work Vikas!

Comment 17

[Vikas Prasad Mahato [:vikas_mahato]]

Thank you!!