Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

[WIP]RAINCATCH-1156 Adding role based check to login #125

Conversation

witmicko
Copy link

@witmicko witmicko commented Sep 19, 2017

Motivation

https://issues.jboss.org/browse/RAINCATCH-1156

Description

Added allowed roles to the server config and cross check them with user roles (intersection of both size > 0)

Progress

  • Passport
  • Unit tests
  • Documentation

Additional Notes

@coveralls
Copy link

coveralls commented Sep 19, 2017

Coverage Status

Changes Unknown when pulling a347dca on witmicko:RAINCATCH-1156_login_roles_check into ** on feedhenry-raincatcher:master**.

@@ -12,14 +12,16 @@ var config = {
"security": {
"adminRole": "admin",
"userRole": "user",
"allowedRoles": ["adminRole"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-1. Roles can be specified directly in the places we need them.
Why we need this in the server?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you agree to rename this to say portalAllowedRoles? I think we should avoid adding this to the client side.

@wtrocki
Copy link
Member

wtrocki commented Sep 19, 2017

@witmicko - We cannot have it this way as we provide role based support on top of the client application. We will need to move from admin and user roles fairly soon.

This means that users will need to use role based check - using our AuthService in the portal client to show dialog if user do not have permission as was suggested in the ticket.
This change needs to work with both keycloak and passworrd.
Using using interface will allow us to do so.

This aproach works but it's not using parts of our framework which we want others to use.

ping @JameelB

@witmicko
Copy link
Author

This means that users will need to use role based check - using our AuthService in the portal client to show dialog if user do not have permission as was suggested in the ticket.

It does exactly, that. Not sure what is the problem here, pull changes locally and try this.

from what I saw so far passport and keycloak differ a lot in implementation.
We cant have role restriction on the client side - what is stopping me from changing it locally or modifying requests in preflight?

@wtrocki
Copy link
Member

wtrocki commented Sep 19, 2017

from what I saw so far passport and keycloak differ a lot in implementation.

They using single interface that it should be used. If that's not possible then that is a bug as PRD clearly states that this should be possible.

@witmicko
Copy link
Author

Closing for now in favour of client-side filtering (server side is still protected)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

4 participants