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

The REST v2 API ignores a user's admin attribute in authorization #37

Closed
bcarlin opened this issue May 20, 2020 · 7 comments
Closed

The REST v2 API ignores a user's admin attribute in authorization #37

bcarlin opened this issue May 20, 2020 · 7 comments
Labels
bug Something isn't working
Milestone

Comments

@bcarlin
Copy link
Member

bcarlin commented May 20, 2020

In the REST v1 API, a user with admin = true as a de facto FULLADMIN role.

However, in the REST v2 API, a user with admin = true as a de facto NOACCESS role.

@fredericBregier
Copy link
Collaborator

@bcarlin I will need more time for this one, since I did not look into REST V2.

@bcarlin
Copy link
Member Author

bcarlin commented May 22, 2020

I don't know what the best way to fix this is.

Authentication is done here, and authorization here.

Maybe it can be done here by skipping authorization checks altogether if the user is an admin...

@fredericBregier
Copy link
Collaborator

It turns out that previous fix (Business fix, in particular ROLE) resolves this issue.
However I added new tests for these for REST V2 (and some extra fix on REST V2 on previous bad usage of Transfer initialization not found until now). (one fix for REST V2)

I'm currently testing it.

fredericBregier added a commit to fredericBregier/Waarp-All that referenced this issue May 22, 2020
- Already fix in previous commit but add some tests to prevent regression
- Also extra missing fix bad usage of initialization of Transfer in Rest V2

Fix issue waarp#37
@fredericBregier
Copy link
Collaborator

See #40

@bcarlin
Copy link
Member Author

bcarlin commented May 26, 2020

It still does not work for me.
I created a user with "is admin" checked

2020-05-26-181536_1119x112_scrot
ZQ

but no request on the v2 rest api works:

$ http --verify no --auth admin:password GET https://127.0.0.1:8088/v2/transfers
HTTP/1.1 403 Forbidden
content-length: 0

fredericBregier added a commit to fredericBregier/Waarp-All that referenced this issue May 27, 2020
- Already fix in previous commit but add some tests to prevent regression
- Also extra missing fix bad usage of initialization of Transfer in Rest V2

Fix issue waarp#37
fredericBregier added a commit to fredericBregier/Waarp-All that referenced this issue May 27, 2020
- Additional fixes on REST V2
- Add some tests to prevent regression
- Also extra missing fix bad usage of initialization of Transfer in Rest V2

Fix issue waarp#37
@fredericBregier
Copy link
Collaborator

Additional fixes in PR

fredericBregier added a commit to fredericBregier/Waarp-All that referenced this issue May 27, 2020
- Additional fixes on REST V2
- Add some tests to prevent regression
- Also extra missing fix bad usage of initialization of Transfer in Rest V2
- Add fix on Proxy Rest test

Fix issue waarp#37
fredericBregier added a commit to fredericBregier/Waarp-All that referenced this issue May 27, 2020
- Additional fixes on REST V2
- Add some tests to prevent regression
- Also extra missing fix bad usage of initialization of Transfer in Rest V2
- Add fix on Proxy Rest test
- Minor fix on default TCP (TCP_NODELAY set to false)

Fix issue waarp#37
@fredericBregier
Copy link
Collaborator

Fixes finalized

fredericBregier added a commit to fredericBregier/Waarp-All that referenced this issue May 28, 2020
- Additional fixes on REST V2
- Add some tests to prevent regression
- Also extra missing fix bad usage of initialization of Transfer in Rest V2
- Add fix on Proxy Rest test
- Minor fix on default TCP (TCP_NODELAY set to false)

Fix issue waarp#37
fredericBregier added a commit to fredericBregier/Waarp-All that referenced this issue May 28, 2020
- Additional fixes on REST V2
- Add some tests to prevent regression
- Also extra missing fix bad usage of initialization of Transfer in Rest V2
- Add fix on Proxy Rest test
- Minor fix on default TCP (TCP_NODELAY set to false)

Fix issue waarp#37
fredericBregier added a commit that referenced this issue May 28, 2020
- Additional fixes on REST V2
- Add some tests to prevent regression
- Also extra missing fix bad usage of initialization of Transfer in Rest V2
- Add fix on Proxy Rest test
- Minor fix on default TCP (TCP_NODELAY set to false)

Fix issue #37
@bcarlin bcarlin added the bug Something isn't working label May 28, 2020
@bcarlin bcarlin added this to the 3.3.4 milestone May 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants