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

app.routing.ts: Making use of isAdmin #840

Merged
merged 3 commits into from Mar 5, 2019

Conversation

agrawalarpit14
Copy link
Contributor

Enforcing isAdmin is true to route to
/#/administration

Part of #336

bkimminich and others added 2 commits March 3, 2019 10:05
Enforcing isAdmin is true to route to
/#/administration

Part of juice-shop#336
@agrawalarpit14
Copy link
Contributor Author

agrawalarpit14 commented Mar 4, 2019

The flow of the challenge can be seen here.
Hi @bkimminich Please let me know if you want any changes to be done and if I can start to write tests?

@bkimminich bkimminich changed the base branch from master to develop March 5, 2019 01:15
@bkimminich
Copy link
Member

bkimminich commented Mar 5, 2019

Looks good to me, but I think other than making the "Admin Section" challenge ⭐⭐ instead of ⭐ there's no additional new challenge to be made out of this, because there's already multiple ways to elevate privileges to get to the Admin page.

@ghost ghost assigned bkimminich Mar 5, 2019
@ghost ghost added the in progress label Mar 5, 2019
@bkimminich
Copy link
Member

bkimminich commented Mar 5, 2019

We could instead make this a new challenge "JWT Tier 0.5" (:star::star::star:) with description something like "Become an admin temporarily. (Do not log in as an actual admin user)" ... To verify we could check if the JWT has isAdmin=true but the mentioned user account is not really an admin in the Users table.

@agrawalarpit14
Copy link
Contributor Author

agrawalarpit14 commented Mar 5, 2019

Hi @bkimminich It sounds great. Then I will go on building the challenge "JWT Tier 0.5" and about this if there is no change can I start writing the tests(solving it in a way where user uses an admin account) ?🤩

@bkimminich
Copy link
Member

Ah, sorry, I just re-watched your video in better resolution and saw that you actually used the same attack needed for "JWT Tier 1" to manipulate the admin flag in the token. So, no new challenge here, I guess... I'll merge your change as is and just increase difficulty for "Admin Access" challenge by 1. Thanks a lot for adding a first step of role-based authorization via JWT to the Juice Shop! 👍

@bkimminich bkimminich merged commit b463d45 into juice-shop:develop Mar 5, 2019
@ghost ghost removed the in progress label Mar 5, 2019
crispy-peppers pushed a commit to crispy-peppers/juice-shop that referenced this pull request Nov 12, 2019
* Block new user registration if registering via MLC
* Allow login with MLC while registration is disabled
@lock
Copy link

lock bot commented Mar 10, 2020

This thread has been automatically locked because it has not had recent activity after it was closed. 🔒 Please open a new issue for regressions or related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants