-
Notifications
You must be signed in to change notification settings - Fork 9
Saml rebase to latest dev code #575
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
Open
timlegge
wants to merge
52
commits into
ctrlo:dev
Choose a base branch
from
timlegge:saml-rebase
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 46 commits
Commits
Show all changes
52 commits
Select commit
Hold shift + click to select a range
cd77535
Show the authentication providers in a table
timlegge 302000f
Working create, update delete authentication providers
timlegge 1bd4e58
Set permissions based on SAML attributes
timlegge 188a6c9
Update the groups from the SAML2 assertion
timlegge 613f762
Allow SAML response to set the groups and permissions
7e907f4
Cleanup and review the previous changes
timlegge bfd21ff
Update the javascript for the provider modals
243a70f
UX and DB changes to associate users to a authentication provider
2adc806
Only allow user to login via SAML if the user provider matches auth id
7f15020
Add provider match error
eebf8a5
Support Encrypted Assertions
411e68d
Enable login via saml login page
fb732ce
Prevent deletion of built in authentication provider
00c1820
Create metadata xml for a provider
bc5d2b8
Fix the fields for enabled and type
63e05e5
Implement IdP initiated login support
6d6f998
Replace hard coded values in metadata
2e2c3fb
Working generation and insertion of cert and key
c390aa7
Switch provider type to a small int - may switch back
a7d8e65
Really fix issue with things in after_create being overwritten
cb0535a
Really fix issue with things in after_create being overwritten
706bf64
Create a unique id for the relaystate - undecided if this is necessary
1fd562b
Never display the private key to the user
c130f82
Show correct value names for Provider Type
e6e4090
Do not allow login if SAML2 provider is disabled
45f862f
Fix issue updating user attributes from assertion
6737ae3
Get hostname from site
8c0e34a
fix acs url in AuthnRequest
6052204
Show error if CA Certificate validation fails
f9135e3
Set the new provider as disabled
a269f40
Redirect to the SAML login page if SAML failed
0c6d249
Fix panic on generating metadata
1b99c7e
Add a FIXME to document after_create issue
fcbd610
Redirect to the saml login page if the user does not exist
fd1dc87
Fix authentication provider missing on create user wizard
timlegge b040083
Fix layout of fields on authentication provider wizard
timlegge e5b52f0
Set the default Authentication provider to builtin
timlegge 64127f8
Don't show editable fields for the builtin authenticator
timlegge 30b036d
Stringify metadata issue installed Net::SAML2 version
timlegge 486dd35
Fix wording issue on error message
timlegge 76c74a6
Do not display fields the user cannot edit
timlegge 6b1a91b
Make sso_xml and sso_url readonly on provider edit
timlegge 47116ee
Set the default provide to builtin if undefined on user creation
timlegge cf1f5a5
Add support for nameid formats
timlegge 1f079cd
The IdP might only accept HTTP-POST - JumpCloud
timlegge b18187b
Fix panic if no GADS Groups re included in the Groups attribute
timlegge ecd073c
Merge remote-tracking branch 'upstream/dev' into modal-changes
droberts-ctrlo 8f1779e
Updates ready for merging
droberts-ctrlo de6b9ce
Update Authentication.pm
droberts-ctrlo 48b88e0
Merge pull request #1 from droberts-ctrlo/modal-changes
timlegge a9feb3c
Improve the Redirect/POST for AuthnRequest
timlegge cdf0865
Change the IdP metadata to a file upload
timlegge File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ use File::Temp qw/ tempfile /; | |
| use GADS::Alert; | ||
| use GADS::Approval; | ||
| use GADS::Audit; | ||
| use GADS::Authentication; | ||
| use GADS::Layout; | ||
| use GADS::Column; | ||
| use GADS::Column::Autocur; | ||
|
|
@@ -153,6 +154,13 @@ sub _update_csrf_token | |
| { session csrf_token => Session::Token->new(length => 32)->get; | ||
| } | ||
|
|
||
| sub get_and_clear_session_value { | ||
| my $id = shift; | ||
| my $value = session $id; | ||
| session $id => undef; | ||
| return $value; | ||
| } | ||
|
|
||
| hook before => sub { | ||
| schema->site_id(undef); | ||
|
|
||
|
|
@@ -475,26 +483,65 @@ get '/saml' => sub { | |
| redirect '/'; | ||
| }; | ||
|
|
||
| post '/saml' => sub { | ||
| post '/saml' => \&saml_post; | ||
|
|
||
| post '/:unique_id/saml' => \&saml_post; | ||
|
|
||
| sub saml_post { | ||
|
|
||
| my $unique_id = route_parameters->get('unique_id'); | ||
| my $relaystate = body_parameters->get('RelayState'); | ||
|
|
||
| my $authentication = schema->resultset('Authentication')->find({saml2_unique_id => $unique_id}) | ||
| or warn "Error finding authentication provider from unique_id" if defined $unique_id; | ||
|
|
||
| $authentication = schema->resultset('Authentication')->find(session 'authentication_id') | ||
| or error "Error finding authentication provider" if !defined $authentication; | ||
|
|
||
| error __"Invalid unique_id in POST request" if $authentication->saml2_unique_id ne $unique_id; | ||
|
|
||
| my $request_id; | ||
| # If the relaystates match this should be an IdP initiated login | ||
| # otherwise get the AuthnReq request id from the session | ||
| if ($authentication->saml2_relaystate ne $relaystate) { | ||
| $request_id = get_and_clear_session_value('request_id'); | ||
| } | ||
|
|
||
| my $saml = GADS::SAML->new( | ||
| request_id => session('request_id'), | ||
| authentication => $authentication, | ||
| request_id => $request_id, | ||
| base_url => request->base, | ||
| ); | ||
|
|
||
| my $callback = $saml->callback( | ||
| saml_response => body_parameters->get('SAMLResponse'), | ||
| defined $authentication->cacert ? | ||
| (cacert => $authentication->cacert) : (), | ||
| defined $authentication->sp_key ? | ||
| (sp_key => $authentication->sp_key) : (), | ||
| defined $relaystate ? (relaystate => $relaystate) : (), | ||
| ); | ||
|
|
||
| my $authentication = schema->resultset('Authentication')->find(session 'authentication_id') | ||
| or error "Error finding authentication provider"; | ||
| my $username; | ||
| if (! defined ($username = $callback->{nameid})) { | ||
| error __"Missing nameid in SAML response"; | ||
| my $msg = $authentication->saml_provider_match_error; | ||
| return forwardHome({ danger => __x($msg, username => $username) }, 'saml_login' ) | ||
| } | ||
|
|
||
| my $username = $callback->{nameid}; | ||
| my $user = schema->resultset('User')->active->search({ username => $username })->next; | ||
|
|
||
| # FIXME: Here we could create the user if the relaystate matches a provider | ||
| if (!defined $user or ($user->provider->id ne $authentication->id)) { | ||
| my $msg = $authentication->saml_provider_match_error; | ||
| $user = undef; | ||
| return forwardHome({ danger => __x($msg, username => $username) }, 'saml_login' ) | ||
| } | ||
|
|
||
| if (!$user) | ||
| { | ||
| my $msg = $authentication->user_not_found_error; | ||
| return forwardHome({ danger => __x($msg, username => $username) }, 'login?password=1' ); | ||
| return forwardHome({ danger => __x($msg, username => $username) }, 'saml_login' ); | ||
| } | ||
|
|
||
| $user->update_attributes($callback->{attributes}); | ||
|
|
@@ -559,6 +606,32 @@ sub _successful_login | |
| } | ||
| } | ||
|
|
||
| any ['get', 'post'] => '/saml_login' => sub { | ||
|
|
||
| my $audit = GADS::Audit->new(schema => schema); | ||
| my $user = logged_in_user; | ||
|
|
||
| # Don't allow login page to be displayed when logged-in, to prevent | ||
| # user thinking they are logged out when they are not | ||
| return forwardHome() if $user; | ||
|
|
||
| my $users = GADS::Users->new(schema => schema, config => config); | ||
| my $output = template 'login_saml' => { | ||
| username => cookie('remember_me'), | ||
| titles => $users->titles, | ||
| organisations => $users->organisations, | ||
| departments => $users->departments, | ||
| teams => $users->teams, | ||
| providers => $users->providers, | ||
| register_text => var('site')->register_text, | ||
| page => 'login', | ||
| body_class => 'p-0', | ||
| container_class => 'login container-fluid', | ||
| main_class => 'login__main row', | ||
| }; | ||
| $output; | ||
| }; | ||
|
|
||
| any ['get', 'post'] => '/login' => sub { | ||
|
|
||
| my $audit = GADS::Audit->new(schema => schema); | ||
|
|
@@ -568,13 +641,24 @@ any ['get', 'post'] => '/login' => sub { | |
| # user thinking they are logged out when they are not | ||
| return forwardHome() if $user; | ||
|
|
||
| # Get authentication provider | ||
| my $enabled = schema->resultset('Authentication')->enabled; | ||
| my $enabled; | ||
| if ((my $saml_user = param('username')) && !query_parameters->get('password')) { | ||
| my $users = GADS::Users->new(schema => schema, config => config); | ||
| my $user_search = $users->user_rs->search({ | ||
| username => $saml_user, | ||
| }); | ||
| my $user = $user_search->next; | ||
| $enabled = schema->resultset('Authentication')->by_id($user->provider->id) if defined $user; | ||
| } | ||
| else { | ||
| # Get authentication provider | ||
| $enabled = schema->resultset('Authentication')->enabled; | ||
| } | ||
|
|
||
| if ($enabled->count == 1 && !query_parameters->get('password')) | ||
| if (defined $enabled && $enabled->count ge 1 && !query_parameters->get('password')) | ||
| { | ||
| my $auth = $enabled->next; | ||
| if ($auth->type eq 'saml2') | ||
| if ($auth->type == 1) | ||
| { | ||
| my $saml = GADS::SAML->new( | ||
| authentication => $auth, | ||
|
|
@@ -662,6 +746,7 @@ any ['get', 'post'] => '/login' => sub { | |
| organisations => $users->organisations, | ||
| departments => $users->departments, | ||
| teams => $users->teams, | ||
| providers => $users->providers, | ||
| register_text => var('site')->register_text, | ||
| page => 'login', | ||
| body_class => 'p-0', | ||
|
|
@@ -731,6 +816,7 @@ any ['get', 'post'] => '/register' => sub { | |
| organisations => $users->organisations, | ||
| departments => $users->departments, | ||
| teams => $users->teams, | ||
| providers => $users->providers, | ||
| register_text => var('site')->register_text, | ||
| page => 'register', | ||
| body_class => 'p-0', | ||
|
|
@@ -770,6 +856,8 @@ any ['get', 'post'] => '/myaccount/?' => require_login sub { | |
| my %update; | ||
| foreach my $field (var('site')->user_fields) | ||
| { | ||
| # FIXME The user should not be able to change their own authentication provider | ||
| next if $field->{name} eq 'provider' && not logged_in_user->permission->{superadmin}; | ||
| next if !$field->{editable}; | ||
| $update{$field->{name}} = param($field->{name}) || undef; | ||
| } | ||
|
|
@@ -790,6 +878,7 @@ any ['get', 'post'] => '/myaccount/?' => require_login sub { | |
| organisation => $users->organisations, | ||
| department_id => $users->departments, | ||
| team_id => $users->teams, | ||
| provider => $users->providers, | ||
| }, | ||
| }; | ||
| }; | ||
|
|
@@ -1504,6 +1593,29 @@ any ['get', 'post'] => '/user_export/?' => require_any_role [qw/useradmin supera | |
| }; | ||
| }; | ||
|
|
||
| any ['get', 'post'] => '/authentication_providers/' => require_any_role [qw/useradmin superadmin/] => sub { | ||
|
|
||
| my @name_ids = ( | ||
| { label_plain => 'emailAddress', value => 'emailAddress' }, | ||
| { label_plain => 'unspecified', value => 'unspecified' }, | ||
| { label_plain => 'X509SubjectName', value => 'X509SubjectName' }, | ||
| { label_plain => 'WindowsDomainQualifiedName', value => 'WindowsDomainQualifiedName' }, | ||
| { label_plain => 'entity', value => 'entity' }, | ||
| { label_plain => 'transient', value => 'transient' }, | ||
| { label_plain => 'persistent', value => 'persistent' }, | ||
| ); | ||
|
|
||
| my $auth = GADS::Authentication->new(schema => schema); | ||
| template 'authentication/providers' => { | ||
| providers => $auth, | ||
| permissions => "permisission", #$auth->permissions, | ||
| values => { | ||
| saml2_nameid => \@name_ids, | ||
| }, | ||
| page => 'system_settings', | ||
| }; | ||
| }; | ||
|
|
||
| any ['get', 'post'] => '/user_overview/' => require_any_role [qw/useradmin superadmin/] => sub { | ||
| my $userso = GADS::Users->new(schema => schema); | ||
|
|
||
|
|
@@ -1538,6 +1650,7 @@ any ['get', 'post'] => '/user_overview/' => require_any_role [qw/useradmin super | |
| organisation => $userso->organisations, | ||
| department_id => $userso->departments, | ||
| team_id => $userso->teams, | ||
| provider => $userso->providers, | ||
| }, | ||
| permissions => $userso->permissions, | ||
| page => 'user', | ||
|
|
@@ -1579,6 +1692,131 @@ any ['get', 'post'] => '/user_requests/' => require_any_role [qw/useradmin super | |
| }; | ||
| }; | ||
|
|
||
| any ['get'] => '/metadata/:id' => require_any_role [qw/useradmin superadmin/] => sub { | ||
| my $id = route_parameters->get('id'); | ||
|
|
||
| my $provider = rset('Authentication')->providers->search({id => $id})->next | ||
| or error __x"Authentication provider id {id} not found", id => $id; | ||
|
|
||
| if (defined $provider) | ||
| { | ||
| if ($provider->type == 1) | ||
| { | ||
| my $saml = GADS::SAML->new( | ||
| authentication => $provider, | ||
| base_url => request->base, | ||
| ); | ||
| response_header 'Content-Disposition' => "attachment; filename=\"saml.xml\""; | ||
| return send_file(\$saml->metadata, content_type => 'application/xml'); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| any ['get', 'post'] => '/authentication_providers/:id' => require_any_role [qw/useradmin superadmin/] => sub { | ||
| my $user = logged_in_user; | ||
| my $userso = GADS::Users->new(schema => schema); | ||
| my $auth = GADS::Authentication->new(schema => schema); | ||
| my $id = route_parameters->get('id'); | ||
| my $audit = GADS::Audit->new(schema => schema, user => $user); | ||
|
|
||
| if (!$id) { | ||
| error __x"Authentication proovider not available"; | ||
| } | ||
|
|
||
| my $editProvider = rset('Authentication')->providers->search({id => $id})->next | ||
| or error __x"Authentication provider id {id} not found", id => $id; | ||
|
|
||
| # FIXME: Is this true in the case of a provider | ||
| # The submit button will still be triggered on a REPLACE_THIS creation, | ||
| # if the user has pressed enter, in which case ignore it | ||
| if (param('submit')) | ||
| { | ||
| my %values = ( | ||
| name => param('name'), | ||
| type => param('type'), | ||
| saml2_firstname => param('saml2_firstname'), | ||
| saml2_surname => param('saml2_surname'), | ||
| xml => param('xml'), | ||
| cacert => param('cacert'), | ||
| # updating sp_cert or sp_key independantly will cause issues | ||
| (defined param('sp_cert') and defined param('sp_key')) ? ( | ||
| sp_cert => param('sp_cert'), | ||
| sp_key => param('sp_key'), | ||
| ) : (), | ||
| saml2_relaystate => param('saml2_relaystate'), | ||
| saml2_groupname => param('saml2_groupname'), | ||
| saml2_nameid => param('saml2_nameid'), | ||
| enabled => param('enabled'), | ||
| ); | ||
| # FIXME: Remove permissions below | ||
| $values{permissions} = [body_parameters->get_all('permission')] | ||
| if logged_in_user->permission->{superadmin}; | ||
| # FIXME: Remove above | ||
|
|
||
| if (process sub { | ||
| # FIXME: permissions note | ||
| # Don't use DBIC update directly, so that permissions etc are updated properly | ||
| $editProvider->update_provider(current_user => logged_in_user, %values); | ||
| }) | ||
| { | ||
| return forwardHome( | ||
| { success => "Authentication Provider has been updated successfully" }, 'authentication_providers/' ); | ||
| } | ||
| } | ||
| elsif (my $delete_id = param('delete')) | ||
| { | ||
| return forwardHome( | ||
| { danger => "You do not have permission to delete an authentication provider" } ) | ||
| if !logged_in_user->permission->{superadmin}; | ||
| my $usero = rset('Authentication')->find($delete_id); | ||
| return forwardHome( | ||
| { danger => "Cannot delete the built in authentication provider" } ) | ||
| if $usero->type == 0; | ||
|
|
||
| # FIXME: Should change this so cannot delete enabled provider for current user | ||
| return forwardHome( | ||
| { danger => "Cannot delete an enabled authentication provider" } ) | ||
| if $usero->enabled; | ||
| # FIXME: Will panic here if a user is still associated with this provider | ||
| # timlegge - fix | ||
| if (process( sub { $usero->retire(current_user => logged_in_user) })) | ||
| { | ||
| #FIXME: fix audit | ||
| $audit->login_change("Authentication Provider ID $delete_id deleted"); | ||
| return forwardHome( | ||
| { success => "Authentication Provider has been updated successfully" }, 'authentication_providers/' ); | ||
| } | ||
| } | ||
|
|
||
| my @types = ( | ||
| { 'label_plain' => 'saml2', value => 'saml2' }, | ||
| { 'label_plain' => 'builtin', value => 'builtin'}, | ||
| ); | ||
|
|
||
| my @name_ids = ( | ||
| { label_plain => 'emailAddress', value => 'emailAddress' }, | ||
| { label_plain => 'unspecified', value => 'unspecified' }, | ||
| { label_plain => 'X509SubjectName', value => 'X509SubjectName' }, | ||
| { label_plain => 'WindowsDomainQualifiedName', value => 'WindowsDomainQualifiedName' }, | ||
| { label_plain => 'entity', value => 'entity' }, | ||
| { label_plain => 'transient', value => 'transient' }, | ||
| { label_plain => 'persistent', value => 'persistent' }, | ||
| ); | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thi si s repeated above at 1596 - Global or constant? |
||
|
|
||
| # FIXME need to revise what is passed to the template | ||
| my $output = template 'authentication/provider_edit' => { | ||
| editprovider => $editProvider, | ||
| groups => GADS::Groups->new(schema => schema)->all, | ||
| values => { | ||
| type => \@types, | ||
| saml2_nameid => \@name_ids, | ||
| }, | ||
| permissions => $userso->permissions, | ||
| page => 'admin', | ||
| }; | ||
| $output; | ||
| }; | ||
|
|
||
| any ['get', 'post'] => '/user/:id' => require_any_role [qw/useradmin superadmin/] => sub { | ||
| my $user = logged_in_user; | ||
| my $userso = GADS::Users->new(schema => schema); | ||
|
|
@@ -1609,6 +1847,7 @@ any ['get', 'post'] => '/user/:id' => require_any_role [qw/useradmin superadmin/ | |
| team_id => param('team_id') || undef, | ||
| account_request => param('account_request'), | ||
| account_request_notes => param('account_request_notes'), | ||
| provider => param('provider') || 1, | ||
| view_limits => [body_parameters->get_all('view_limits')], | ||
| groups => [body_parameters->get_all('groups')], | ||
| ); | ||
|
|
@@ -1646,6 +1885,7 @@ any ['get', 'post'] => '/user/:id' => require_any_role [qw/useradmin superadmin/ | |
| organisation => $userso->organisations, | ||
| department_id => $userso->departments, | ||
| team_id => $userso->teams, | ||
| provider => $userso->providers, | ||
| }, | ||
| permissions => $userso->permissions, | ||
| page => 'user', | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section is repeated at 1796 - Use Global or constants?