Saml rebase to latest dev code #575
Conversation
Create unique URLs for each provider
Essentially the provide was created - after_create would run and then the provide was updated with the original data
Essentially the provide was created - after_create would run and then the provide was updated with the original data
Allow admin to overwrite
|
Tested the rebase and fixed 3 issues: Could not create a new user via the wizard Fix authentication provider missing on create user wizard The field layout was somewhat confusing This caused it to default to the saml_login on logout so essentially kept logging you in. Set the default Authentication provider to builtin I will also plan to change the views/authentication/provider_edit.tt to not show any fields for the "builtin" authentication provider. |
timlegge
left a comment
timlegge
left a comment
There was a problem hiding this comment.
Comments I thought I had sent previously
| # FIXME: SAML should be able to set groups | ||
| # error __"You do not have permission to set global user permissions" | ||
| # if !$current_user->permission->{superadmin}; | ||
| # |
There was a problem hiding this comment.
This needs a review. We will want SAML assertions to be able to update the groups on login if included in the assertion. However, we need to ensure that only superadmin and the SAML assertion can make those changes.
| 'GADS-SuperAdmin' => 'superadmin', | ||
| 'GADS-UserAdmin' => 'useradmin', | ||
| 'GADS-Audit' => 'audit', | ||
| ); |
There was a problem hiding this comment.
Need to add those permission mappings to the UI?
| { | ||
| # FIXME: SAML should be able to set groups | ||
| # error __"You do not have permission to set global user permissions" | ||
| # if !$self->permission->{superadmin}; |
There was a problem hiding this comment.
This needs a review. We will want SAML assertions to be able to update the permissions on login if included in the assertion. However, we need to ensure that only superadmin and the SAML assertion can make those changes.
| my @permissions; | ||
| for my $permission (@{$attributes->{$at}}) { | ||
| # FIXME: hard coded permission? | ||
| push @permissions, $permission_map{$permission} if defined $permission_map{$permission} and $permission =~ /^GADS-/; |
There was a problem hiding this comment.
Hard coded permissions "GADS-..."
| my @groups; | ||
| # Automatically update the groups for the user from the SAML2 attributes | ||
| for my $group (@{$attributes->{$at}}) { | ||
| next if defined $permission_map{$group}; |
There was a problem hiding this comment.
A permision_map group is one of:
'GADS-SuperAdmin' => 'superadmin',
'GADS-UserAdmin' => 'useradmin',
'GADS-Audit' => 'audit',
They should be ignored for creating groups - Assertions only have groups. This allows us to use groups for permissions and groups
There was a problem hiding this comment.
Need to reset the permissions to the original
| { label_plain => 'entity', value => 'entity' }, | ||
| { label_plain => 'transient', value => 'transient' }, | ||
| { label_plain => 'persistent', value => 'persistent' }, | ||
| ); |
There was a problem hiding this comment.
This section is repeated at 1796 - Use Global or constants?
| { label_plain => 'entity', value => 'entity' }, | ||
| { label_plain => 'transient', value => 'transient' }, | ||
| { label_plain => 'persistent', value => 'persistent' }, | ||
| ); |
There was a problem hiding this comment.
Thi si s repeated above at 1596 - Global or constant?
| $self->clear_has_group; | ||
| $self->has_group; | ||
| } | ||
| } |
There was a problem hiding this comment.
There is likely a much better way to do this
Modal changes
Note that this does not fully support a post only IDP It cheats by trying a Redirect to the POST address It needs a template to generate a page for the user as an auto post request or a manual submit button
timlegge
changed the title
2 participants
I wanted to get this finally into a PR. It took some fiddling to get this rebased on my local machine so I have not tested the code again on the test system but I plan to try that tomorrow.
I could use some assistance with:
bfd21ff Update the javascript for the provider modals
It requires the js to be regenerated.
Please review it to see if any of the changes for the js will cause an issue.