routing security issue allows to visit protected routes

xetys commented 7 years ago

Overview of the issue

as in angularJS we are using the data.authorities property in routes in Angular2+ to protect particular routes based on the users role. However this feature seems not to work properly in the ng2 client. A user with role USER might manually type "/user-management" as URL and successfully retrieve a list of users. There are limitation for the user to perform real harmful actions, such as deleting / creating users, changing logger properties, as the backend has also a layer of protection.

So this is just "unbeaty" rather than a security issue. However the proper behaviour should be a redirect to "Access Denied" page, as it is done to anulgar JS

Motivation for or Use Case

One possibile motivation is to redirect a user to login for all pages, by placing 'ROLE_USER' to home route, and therefor perform a auto login popup open in the Access Denied page to login the user..

Reproduce the error

generate any JHipster application with users an angular client, and try to hit protected routes as "/user-management"

Suggest a Fix

the router should redirect to access denied page, if the data.authorities contains authorities, which are not present in the principals roles

JHipster Version(s)

tested on current master

[x] Checking this box is mandatory (this is just to show you read everything)

xetys commented 7 years ago

in the meantime I found a solution for this. It's just to hacky now to PR it yet

jdubois commented 7 years ago

In fact there are 2 issues:

The "user" can see the list of users, but can't modify them -> this is "normal", at least it works as expected. The server-side is correctly protected.
On the client side, this is just a UI check, and the routes do not appear on the menu when you don't have the permissions. Then, indeed, we should also block the routes when the user doesn't have the permissions - that's what we were doing in Angular 1, and that's indeed much nicer.

flaviencathala commented 7 years ago

I made two different fixes for this, I personnaly think the clean one is a lot better.

jdubois commented 7 years ago

This is really "tricky"! Can't believe it :-)

deepu105 commented 7 years ago

Lets not encourage people use tricks like these as its very easy to break. And prople add multiple underscores to a method name for a reason so that no body can find and use it :P

Thanks & regards, Deepu

On 20 Feb 2017 3:36 p.m., "Julien Dubois" notifications@github.com wrote:

This is really "tricky"! Can't believe it :-)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/issues/5249#issuecomment-281094815, or mute the thread https://github.com/notifications/unsubscribe-auth/ABDlF6kPeHmmxfqRcjM2_58r2mWLpJ7Gks5reaS_gaJpZM4MFeCb .

xetys commented 7 years ago

of course we take the first one....but the tricky one is cool 😄

however I found this issue actually from another issue, which might be fixed right in place: if you give the home.route any role (which means you should be authenticated), it doesn't deny.

jdubois commented 7 years ago

Closing as #5261 is merged

jhipster / generator-jhipster