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.

Sign up for GitHub

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

Jump to bottom

Fix: keep local groups when there is no group mapping set up #806

Merged
merged 2 commits into from
Jan 30, 2024
Merged

Fix: keep local groups when there is no group mapping set up #806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ae07a7d
tests(groups): test keeping local groups /wo mapping
blizzz Jan 29, 2024
579bf97
fix(groups): do not handle groups without mapping
blizzz Jan 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions lib/GroupManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,12 +115,19 @@ private function getGroupsToAdd(array $samlGroupNames, array $assignedGroupIds):
}

public function handleIncomingGroups(IUser $user, array $samlGroupNames): void {
$id = $this->settings->getProviderId();
$groupMapping = $this->settings->get($id)['saml-attribute-mapping-group_mapping'] ?? null;
if ($groupMapping === null || $groupMapping === '') {
// When no group mapping is set up, it is not our business
return;
}

$this->updateUserGroups($user, $samlGroupNames);
// TODO: drop following line with dropping NC 28 support
$this->evaluateGroupMigrations($samlGroupNames);
}

public function updateUserGroups(IUser $user, array $samlGroupNames): void {
protected function updateUserGroups(IUser $user, array $samlGroupNames): void {
$this->translateGroupToIds($samlGroupNames);
$assignedGroups = $this->groupManager->getUserGroups($user);
$assignedGroupIds = array_map(function (IGroup $group) {
Expand Down Expand Up @@ -250,7 +257,7 @@ protected function hasSamlBackend(IGroup $group): bool {
return in_array('user_saml', $group->getBackendNames());
}

public function evaluateGroupMigrations(array $groups): void {
protected function evaluateGroupMigrations(array $groups): void {
$candidateInfo = $this->config->getAppValue('user_saml', self::LOCAL_GROUPS_CHECK_FOR_MIGRATION, '');
if ($candidateInfo === '') {
return;
Expand Down
40 changes: 38 additions & 2 deletions tests/integration/features/Shibboleth.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,6 @@ Feature: Shibboleth
|j_username|j_password|_eventId_proceed|
|student1 |password | |
And The response should be a SAML redirect page that gets submitted
#And I should be redirected to "http://localhost:8080/index.php/apps/dashboard/"
And The user value "groups" should be "Astrophysics,SAML_Students"
And the group backend of "Astrophysics" should be "Database"
And Then the group backend of "Astrophysics" must not be "user_saml"
Expand Down Expand Up @@ -365,7 +364,6 @@ Feature: Shibboleth
|j_username|j_password|_eventId_proceed|
|student1 |password | |
And The response should be a SAML redirect page that gets submitted
#And I should be redirected to "http://localhost:8080/index.php/apps/dashboard/"
And The user value "groups" should be "SAML_Astrophysics,SAML_Students"
And the group backend of "Archaeologists" should be "Database"
And Then the group backend of "Archaeologists" must not be "user_saml"
Expand Down Expand Up @@ -417,3 +415,41 @@ Feature: Shibboleth
|student2 |password | |
And The response should be a SAML redirect page that gets submitted
And The user value "groups" should be "SAML_Students"

Scenario: Not migrating a local group to SAML backend when no mapping is configured
Given The setting "type" is set to "saml"
And The setting "general-uid_mapping" is set to "urn:oid:0.9.2342.19200300.100.1.1"
And The setting "idp-entityId" is set to "https://shibboleth-integration-nextcloud.localdomain/idp/shibboleth"
And The setting "idp-singleSignOnService.url" is set to "https://localhost:4443/idp/profile/SAML2/Redirect/SSO"
And The setting "idp-singleLogoutService.url" is set to "https://localhost:4443/idp/profile/Logout"
And The setting "idp-x509cert" is set to "MIIDnTCCAoWgAwIBAgIUGPx9uPjCu7c172IUgV84Tm94pBcwDQYJKoZIhvcNAQEL BQAwNzE1MDMGA1UEAwwsc2hpYmJvbGV0aC1pbnRlZ3JhdGlvbi1uZXh0Y2xvdWQu bG9jYWxkb21haW4wHhcNMTcwMTA0MTAxMTI3WhcNMzcwMTA0MTAxMTI3WjA3MTUw MwYDVQQDDCxzaGliYm9sZXRoLWludGVncmF0aW9uLW5leHRjbG91ZC5sb2NhbGRv bWFpbjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKH+bPu45tk8/JRk XOxkyfbxocWZlY4mRumEUxscd3fn0oVzOrdWbHH7lCZV4bus4KxvJljc0Nm2K+Zr LoiRUUnf/LQ4LlehWVm5Kbc4kRgOXS0iGZN3SslAWPKyIg0tywg+TLOBPoS6EtST 1WuYg1JPMFxPfeFDWQ0dQYPlXIJWBFh6F2JMTb0FLECqA5l/ryYE13QisX5l+Mqo 6y3Dh7qIgaH0IJNobXoAcEWza7Kb2RnfhZRh9e0qjZIwBqTJUFM/6I86RYXn829s INUvYQQbez6VkGTdUQJ/GuXb/dD5sMQfOyK8hrRY5MozOmK32cz3JaAzSXpiSRS9 NxFwvicCAwEAAaOBoDCBnTAdBgNVHQ4EFgQUKn8+TV0WXSDeavvF0M8mWn1o8ukw fAYDVR0RBHUwc4Isc2hpYmJvbGV0aC1pbnRlZ3JhdGlvbi1uZXh0Y2xvdWQubG9j YWxkb21haW6GQ2h0dHBzOi8vc2hpYmJvbGV0aC1pbnRlZ3JhdGlvbi1uZXh0Y2xv dWQubG9jYWxkb21haW4vaWRwL3NoaWJib2xldGgwDQYJKoZIhvcNAQELBQADggEB ABI6uzoIeLZT9Az2KTlLxIc6jZ4MDmhaVja4ZuBxTXEb7BFLfeASEJmQm1wgIMOn pJId3Kh3njW+3tOBWKm7lj8JxVVpAu4yMFSoQGPaVUgYB1AVm+pmAyPLzfJ/XGhf esCU2F/b0eHWcaIb3x+BZFX089Cd/PBtP84MNXdo+TccibxC8N39sr45qJM/7SC7 TfDYU0L4q2WZHJr4S7+0F+F4KaxLx9NzCvN4h6XaoWofZWir2iHO4NzbrVQGC0ei QybS/neBfni4A2g1lyzCb6xFB58JBvNCn7AAnDJULOE7S5XWUKsDAQVQrxTNkUq7 pnhlCQqZDwUdgmIXd1KB1So="
And The setting "security-authnRequestsSigned" is set to "1"
And The setting "security-wantAssertionsEncrypted" is set to "1"
And The setting "sp-x509cert" is set to "-----BEGIN CERTIFICATE-----MIIC+zCCAeOgAwIBAgIJAIgZuvWDBIrdMA0GCSqGSIb3DQEBBQUAMBQxEjAQBgNVBAMMCWxvY2FsaG9zdDAeFw0xNzAxMDQxMTM5MjFaFw0yNzAxMDIxMTM5MjFaMBQxEjAQBgNVBAMMCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAN3ESWaDH1JiJTy9yRJQV7kahPOxgBkIH2xwcYDL1k9deKNhSKLx7aGfxE244+HBcC6WLHKVUnOm0ld2qxQ4bMYiJXzZuqL67r07L5wxGAssv12lO92qohGmlHy3+VzRYUBmovu6upqOv3R2F8HBbo7Jc7Hvt7hOEJn/jPuFuF/fHit3mqU8l6IkrIZjpaW8T9fIWOXRq98U4+hkgWpqEZWsqlfE8BxAs9DeIMZab0GxO9stHLp+GYKx10uE4ezFcaDS8W+g2C8enCTt1HXGvcnj4o5zkC1lITGvcFTsiFqfIWyXeSufcxdc0W7HoG6J3ks0WJyK38sfFn0t2Ao6kX0CAwEAAaNQME4wHQYDVR0OBBYEFAoJzX6TVYAwC1GSPe6nObBG54zaMB8GA1UdIwQYMBaAFAoJzX6TVYAwC1GSPe6nObBG54zaMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBAJia9R70uXdUZtgujUPjLas4+sVajzlBFmqhBqpLAo934vljf9HISsHrPtdBcbS0d0rucqXwabDf0MlR18ksnT/NYpsTwMbMx76CrXi4zYEEW5lISKEO65aIkzVTcqKWSuhjtSnRdB6iOLsFiKmNMWXaIKMR5T0+AbR9wdQgn08W+3EEeHGvafVQfE3STVsSgNb1ft7DvcSUnfPXGU7KzvmTpZa0Hfmc7uY4vpdEEhLAdRhgLReS7USZskov7ooiPSoD+JRFi2gM4klBxTemHdNUa9oFnHMXuYKOkLbkgFvHxyy+QlLq2ELQTga5e7I83ZyOfGctyf8Ul6vGw10vbQ=-----END CERTIFICATE-----"
And The setting "sp-privateKey" is set to "-----BEGIN RSA PRIVATE KEY-----MIIEpAIBAAKCAQEA3cRJZoMfUmIlPL3JElBXuRqE87GAGQgfbHBxgMvWT114o2FIovHtoZ/ETbjj4cFwLpYscpVSc6bSV3arFDhsxiIlfNm6ovruvTsvnDEYCyy/XaU73aqiEaaUfLf5XNFhQGai+7q6mo6/dHYXwcFujslzse+3uE4Qmf+M+4W4X98eK3eapTyXoiSshmOlpbxP18hY5dGr3xTj6GSBamoRlayqV8TwHECz0N4gxlpvQbE72y0cun4ZgrHXS4Th7MVxoNLxb6DYLx6cJO3Udca9yePijnOQLWUhMa9wVOyIWp8hbJd5K59zF1zRbsegboneSzRYnIrfyx8WfS3YCjqRfQIDAQABAoIBAQC5CQAdcqZ9vLpJNilBCJxJLCFmm+HAAREHD8MErg9A5UK1P4S1wJp/0qieGPi68wXBOTgY2xKSwMycgb04/+NyZidVRu388takOW/+KNBg8pMxdZ6/05GqnI0kivSbR3CXpYuz8hekwhpo9+fWmKjApsHL47ItK6WaeKmPbAFsq1YJGzfp/DXg7LIvh9GA3C1LWWGV7SuCGOyX/2Moi8xRa7qBtH4hDo/0NRhTx7zjYjlBgNEr330pJUopc3+AtHE40R+xMr2zkGvq9RsCZxYxD2VWbLwQW0yNjWmQ2OTuMgJJvk2+N73QLHcB+tea82ZTszsNzRS9DLtc6qbsKEPZAoGBAO78U3vEuRyY56f/1hpo0xuCDwOkWGzgBQWkjJl6dlyVz/zKkhXBHpEYImyt8XRN0W3iGZYpZ2hCFJGTcDp32R6UiEyGLz0Uc8R/tva/TiRVW1FdNczzSHcB24b9OMK4vE9JLs8mA8Rp8YBgtLr5DDuMfYt/a/rZJbg/HIfIN98nAoGBAO2OInCX93t2I6zzRPIqKtI6q6FYNp64VIQjvw9Y8l0x3IdJZRP9H5C8ZhCeYPsgEqTXcXa4j5hL4rQzoUtxfxflBUUH60bcnd4LGaTCMYLS14G011E3GZlIP0sJi5OjEhy8fq3zt6jVzS9V/lPHB8i+w1D7CbPrMpW7B3k32vC7AoGAX/HvdkYhZyjAAEOG6m1hK68IZhbp5TP+8CgCxm9S65K9wKh3A8LXibrdvzIKOP4w8WOPkCipOkMlTNibeu24vj01hztr5aK7Y40+oEtnjNCz67N3MQQO+LBHOSeaTRqrh01DPKjvZECAU2D/zfzEe3fIw2Nxr3DUYub7hkvMmosCgYAzxbVVypjiLGYsDDyrdmsstCKxoDMPNmcdAVljc+QmUXaZeXJw/8qAVb78wjeqo1vM1zNgR2rsKyW2VkZB1fN39q7GU6qAIBa7zLmDAduegmr7VrlSduq6UFeS9/qWa4TIBICrUqFlR2tXdKtgANF+e6y/mmaL8qdsoH1JetXZfwKBgQC1vscRpdAXivjOOZAh+mzJWzS4BUl4CTJLYYIuOEXikmN5g0EdV2fhUEdkewmyKnXHsd0x83167bYgpTDNs71jUxDHy5NXlg2qIjLkf09X9wr19gBzDApfWzfh3vUqttyMZuQMLVNepGCWM2vjlY9KGl5OvZqY6d+7yO0mLV9GmQ==-----END RSA PRIVATE KEY-----"
And The setting "security-wantAssertionsSigned" is set to "1"
And The setting "localGroupsCheckForMigration" is set to '{\"dropAfter\":9223372036854775807,\"groups\":[\"Archaeologists\", \"Astrophysics\",\"Students\"]}'
And The setting "saml-attribute-mapping-email_mapping" is set to "urn:oid:0.9.2342.19200300.100.1.3"
And The setting "saml-attribute-mapping-displayName_mapping" is set to "urn:oid:2.5.4.42 urn:oid:2.5.4.4"
And the local group "Archaeologists" is created
# Initial login so the user is known and belonging to SAML, directly followed by a logout
And I send a GET request to "http://localhost:8080/index.php/login"
And I should be redirected to "https://localhost:4443/idp/profile/SAML2/Redirect/SSO"
And I send a POST request to "https://localhost:4443/idp/profile/SAML2/Redirect/SSO?execution=e1s1" with the following data
|j_username|j_password|_eventId_proceed|
|student1 |password | |
And The response should be a SAML redirect page that gets submitted
And I send a GET request with requesttoken to "http://localhost:8080/index.php/apps/user_saml/saml/sls"
And the cookies are cleared
And the user "student1" is added to the group "Archaeologists"
# Now for the actual test
When I send a GET request to "http://localhost:8080/index.php/login"
Then I should be redirected to "https://localhost:4443/idp/profile/SAML2/Redirect/SSO"
And I send a POST request to "https://localhost:4443/idp/profile/SAML2/Redirect/SSO?execution=e1s1" with the following data
|j_username|j_password|_eventId_proceed|
|student1 |password | |
And The response should be a SAML redirect page that gets submitted
And The user value "groups" should be "Archaeologists"
And the group backend of "Archaeologists" should be "Database"
And Then the group backend of "Archaeologists" must not be "user_saml"
And I expect no background job for class "OCA\\User_SAML\\Jobs\\MigrateGroups"
2 changes: 1 addition & 1 deletion tests/unit/GroupManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public function testUpdateUserGroups() {
->with($user, ['groupC']);

// assert SAML provides user groups groupB and groupC
$this->ownGroupManager->updateUserGroups($user, ['groupB', 'groupC']);
$this->invokePrivate($this->ownGroupManager, 'updateUserGroups', [$user, ['groupB', 'groupC']]);
}

public function testUnassignUserFromGroups() {
Expand Down
Loading