Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team. by fisx · Pull Request #1755 · wireapp/wire-server

fisx · 2021-09-10T15:22:46Z

Support using a single IDP with a single EntityID to set up two teams. sets up a migration, and makes teamID + EntityID unique, rather than relying on EntityID to be unique. Required to support multiple teams in environments where the IDP software cannot present anything but one EntityID (E.G.: DualShield).

Checklist

The PR Title explains the impact of the change.
The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
If end-points have been added or changed: the endpoint / config-flag checklist (see Wire-employee only backend wiki page) has been followed.
If a schema migration has been added, I ran make git-add-cassandra-schema to update the cassandra schema documentation.
changelog.d contains the following bits of information:
- A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

fisx · 2021-09-10T15:39:08Z

Failures:

  test-integration/Test/Spar/APISpec.hs:244:11: 
  1) WireIdPAPIV1.Spar.API, POST /sso/finalize-login, access granted, happy flow, responds with a very peculiar 'allowed' HTTP response
       "<?xml version=\"1.0\" encoding=\"UTF-8\"?><!DOCTYPE html PUBLIC \"-//W3C//DTD XHTML 1.1//EN\" \"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd\"><html xml:lang=\"en\" xmlns=\"http://www.w3.org/1999/xhtml\"><head>  <title>wire:sso:error:forbidden</title>   <script type=\"text/javascript\">       const receiverOrigin = '*';       window.opener.postMessage({\"payload\":{\"errors\":[\"Audience mismatch: we are http://localhost:8088/sso/finalize-login, they expect one of [http://localhost:8088/sso/finalize-login/ca3474ed-3774-4185-b770-65a20ecb5220]\"],\"label\":\"forbidden\"},\"type\":\"AUTH_ERROR\"}, receiverOrigin);   </script></head></html>" does not contain "<title>wire:sso:success</title>"

The most interesting part is this:

["Audience mismatch: we are http://localhost:8088/sso/finalize-login, they expect one of [http://localhost:8088/sso/finalize-login/ca3474ed-3774-4185-b770-65a20ecb5220]"]

So for IdPs expecting the old behavior, the auth request names the new finalize-login path. That shouldn't be too hard to fix, no?

This reverts commit 17c22336940529a014d649e3d17eed6de6f1c05d.

This reverts commit 8c4316a034f3d248e8eeb4f1060a3ade21c9d467.

This reverts commit e0853d3a4a4a6ddbaf189eeff5c7633f62b016cf.

... as long as they really live in different teams!

More error info when looking up idps.

libs/wire-api/src/Wire/API/User/IdentityProvider.hs

services/spar/src/Spar/API.hs

services/spar/src/Spar/App.hs

services/spar/src/Spar/Data.hs

services/spar/test-integration/Util/Types.hs

services/spar/test-integration/Test/Spar/APISpec.hs

julialongtin

reviewed over wire. was fun!

jschaul · 2021-09-13T19:35:41Z

There are 4 failing tests (consistently across two CI runs):

Failures:


  test-integration/Test/Spar/APISpec.hs:138:7: 

  1) WireIdPAPIV1.Spar.API.metadata metadata (legacy)

       predicate failed on: Response {responseStatus = Status {statusCode = 200, statusMessage = "OK"}, responseVersion = HTTP/1.1, responseHeaders = [("Transfer-Encoding","chunked"),("Date","Mon, 13 Sep 2021 19:23:48 GMT"),("Server","Warp/3.3.13"),("Cache-Control","no-cache, no-store"),("Pragma","no-cache"),("Content-Type","application/xml")], responseBody = Just "<md:EntityDescriptor xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:samla=\"urn:oasis:names:tc:SAML:2.0:assertion\" xmlns:samlm=\"urn:oasis:names:tc:SAML:2.0:metadata\" xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\" entityID=\"http://spar:8080/sso/finalize-login\" xmlns:md=\"urn:oasis:names:tc:SAML:2.0:metadata\"><md:SPSSODescriptor ID=\"_bf1b0086-392f-4a30-8380-71adca8e1471\" WantAssertionsSigned=\"true\" cacheDuration=\"PT2592000S\" protocolSupportEnumeration=\"urn:oasis:names:tc:SAML:2.0:protocol\" validUntil=\"2022-09-13T19:23:48.7276188Z\"><md:Organization><md:OrganizationName xml:lang=\"EN\">spar</md:OrganizationName><md:OrganizationDisplayName xml:lang=\"EN\">spar</md:OrganizationDisplayName><md:OrganizationURL xml:lang=\"EN\">http://spar:8080/sso/finalize-login</md:OrganizationURL></md:Organization><md:ContactPerson contactType=\"support\"><md:Company>Example Company</md:Company><md:EmailAddress>email:backend+spar@wire.com</md:EmailAddress></md:ContactPerson><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:entity</md:NameIDFormat><md:AssertionConsumerService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\" Location=\"http://spar:8080/sso/finalize-login\" index=\"0\" isDefault=\"true\"/></md:SPSSODescriptor></md:EntityDescriptor>", responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}


  To rerun use: --match "/WireIdPAPIV1/Spar.API/metadata/metadata (legacy)/"


  test-integration/Test/Spar/APISpec.hs:150:7: 

  2) WireIdPAPIV1.Spar.API.metadata metadata (with team-id)

       predicate failed on: Response {responseStatus = Status {statusCode = 200, statusMessage = "OK"}, responseVersion = HTTP/1.1, responseHeaders = [("Transfer-Encoding","chunked"),("Date","Mon, 13 Sep 2021 19:23:48 GMT"),("Server","Warp/3.3.13"),("Cache-Control","no-cache, no-store"),("Pragma","no-cache"),("Content-Type","application/xml")], responseBody = Just "<md:EntityDescriptor xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:samla=\"urn:oasis:names:tc:SAML:2.0:assertion\" xmlns:samlm=\"urn:oasis:names:tc:SAML:2.0:metadata\" xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\" entityID=\"http://spar:8080/sso/finalize-login/3f1a01e2-1092-11ec-846d-3344b58840fe\" xmlns:md=\"urn:oasis:names:tc:SAML:2.0:metadata\"><md:SPSSODescriptor ID=\"_0665b2c1-ea4b-441d-8987-fc9ebcec0dc1\" WantAssertionsSigned=\"true\" cacheDuration=\"PT2592000S\" protocolSupportEnumeration=\"urn:oasis:names:tc:SAML:2.0:protocol\" validUntil=\"2022-09-13T19:23:48.7315704Z\"><md:Organization><md:OrganizationName xml:lang=\"EN\">spar</md:OrganizationName><md:OrganizationDisplayName xml:lang=\"EN\">spar</md:OrganizationDisplayName><md:OrganizationURL xml:lang=\"EN\">http://spar:8080/sso/finalize-login/3f1a01e2-1092-11ec-846d-3344b58840fe</md:OrganizationURL></md:Organization><md:ContactPerson contactType=\"support\"><md:Company>Example Company</md:Company><md:EmailAddress>email:backend+spar@wire.com</md:EmailAddress></md:ContactPerson><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:entity</md:NameIDFormat><md:AssertionConsumerService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\" Location=\"http://spar:8080/sso/finalize-login/3f1a01e2-1092-11ec-846d-3344b58840fe\" index=\"0\" isDefault=\"true\"/></md:SPSSODescriptor></md:EntityDescriptor>", responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}


  To rerun use: --match "/WireIdPAPIV1/Spar.API/metadata/metadata (with team-id)/"


  test-integration/Test/Spar/APISpec.hs:138:7: 

  3) WireIdPAPIV2.Spar.API.metadata metadata (legacy)

       predicate failed on: Response {responseStatus = Status {statusCode = 200, statusMessage = "OK"}, responseVersion = HTTP/1.1, responseHeaders = [("Transfer-Encoding","chunked"),("Date","Mon, 13 Sep 2021 19:25:52 GMT"),("Server","Warp/3.3.13"),("Cache-Control","no-cache, no-store"),("Pragma","no-cache"),("Content-Type","application/xml")], responseBody = Just "<md:EntityDescriptor xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:samla=\"urn:oasis:names:tc:SAML:2.0:assertion\" xmlns:samlm=\"urn:oasis:names:tc:SAML:2.0:metadata\" xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\" entityID=\"http://spar:8080/sso/finalize-login\" xmlns:md=\"urn:oasis:names:tc:SAML:2.0:metadata\"><md:SPSSODescriptor ID=\"_983969b0-7bdb-428a-9ef4-99ec5b376a24\" WantAssertionsSigned=\"true\" cacheDuration=\"PT2592000S\" protocolSupportEnumeration=\"urn:oasis:names:tc:SAML:2.0:protocol\" validUntil=\"2022-09-13T19:25:53.0600829Z\"><md:Organization><md:OrganizationName xml:lang=\"EN\">spar</md:OrganizationName><md:OrganizationDisplayName xml:lang=\"EN\">spar</md:OrganizationDisplayName><md:OrganizationURL xml:lang=\"EN\">http://spar:8080/sso/finalize-login</md:OrganizationURL></md:Organization><md:ContactPerson contactType=\"support\"><md:Company>Example Company</md:Company><md:EmailAddress>email:backend+spar@wire.com</md:EmailAddress></md:ContactPerson><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:entity</md:NameIDFormat><md:AssertionConsumerService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\" Location=\"http://spar:8080/sso/finalize-login\" index=\"0\" isDefault=\"true\"/></md:SPSSODescriptor></md:EntityDescriptor>", responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}


  To rerun use: --match "/WireIdPAPIV2/Spar.API/metadata/metadata (legacy)/"


  test-integration/Test/Spar/APISpec.hs:150:7: 

  4) WireIdPAPIV2.Spar.API.metadata metadata (with team-id)

       predicate failed on: Response {responseStatus = Status {statusCode = 200, statusMessage = "OK"}, responseVersion = HTTP/1.1, responseHeaders = [("Transfer-Encoding","chunked"),("Date","Mon, 13 Sep 2021 19:25:52 GMT"),("Server","Warp/3.3.13"),("Cache-Control","no-cache, no-store"),("Pragma","no-cache"),("Content-Type","application/xml")], responseBody = Just "<md:EntityDescriptor xmlns:samlp=\"urn:oasis:names:tc:SAML:2.0:protocol\" xmlns:samla=\"urn:oasis:names:tc:SAML:2.0:assertion\" xmlns:samlm=\"urn:oasis:names:tc:SAML:2.0:metadata\" xmlns:ds=\"http://www.w3.org/2000/09/xmldsig#\" entityID=\"http://spar:8080/sso/finalize-login/3f1a01e2-1092-11ec-846d-3344b58840fe\" xmlns:md=\"urn:oasis:names:tc:SAML:2.0:metadata\"><md:SPSSODescriptor ID=\"_b488dd1e-a95e-4b73-8e28-14a5d8c4c173\" WantAssertionsSigned=\"true\" cacheDuration=\"PT2592000S\" protocolSupportEnumeration=\"urn:oasis:names:tc:SAML:2.0:protocol\" validUntil=\"2022-09-13T19:25:53.0641463Z\"><md:Organization><md:OrganizationName xml:lang=\"EN\">spar</md:OrganizationName><md:OrganizationDisplayName xml:lang=\"EN\">spar</md:OrganizationDisplayName><md:OrganizationURL xml:lang=\"EN\">http://spar:8080/sso/finalize-login/3f1a01e2-1092-11ec-846d-3344b58840fe</md:OrganizationURL></md:Organization><md:ContactPerson contactType=\"support\"><md:Company>Example Company</md:Company><md:EmailAddress>email:backend+spar@wire.com</md:EmailAddress></md:ContactPerson><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:2.0:nameid-format:entity</md:NameIDFormat><md:AssertionConsumerService Binding=\"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\" Location=\"http://spar:8080/sso/finalize-login/3f1a01e2-1092-11ec-846d-3344b58840fe\" index=\"0\" isDefault=\"true\"/></md:SPSSODescriptor></md:EntityDescriptor>", responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}


  To rerun use: --match "/WireIdPAPIV2/Spar.API/metadata/metadata (with team-id)/"


Randomized with seed 2095268154


Finished in 253.0226 seconds

507 examples, 4 failures, 72 pending

fisx · 2021-09-13T19:53:00Z

(at least some of) the failures are happening here:

- "...Location=\"http://spar:8080/sso/finalize-login\"..."
+ "...Location=\"http://localhost:8088/sso/finalize-login\"..."

That's why I couldn't reproduce this locally. I'll think of a cheap way to abstract over the spar hostname.

(not one that we falsely assume will always be configured.)

fisx · 2021-09-13T21:13:41Z

I also think this is ready to merge now, if concourse is done. But @julialongtin I wouldn't mind a second opinion on 379c56c...

services/spar/src/Spar/API.hs

* document #1755 in spar-braindump.md * Fix: do not fail on registering same IdP entityID for *third* team. * Fix: do not allow authenticating same `UserRef` for two different teams. (didn't work before, not it throws an error.) * Flip the order of two commutative db calls. (idp by issuer is stored either in one or the other table, never in both, so this doesn't matter except for considerations as to which is most likely to fail and should therefore be tried last.) * Simplify code. `guardReplaceeV2` was never called, and what it checks is already checked in the beginning of `validateNewIdP`; `guardSameTeam` was either testing a tautology or a falsehood, depending on the call side, so it was easy to simplify. * Haddocks. * Don't use `-` in query params; use `_` instead. * Add failing integration test. * Cleanup: obey (optional) field order in cassandra insert. * More error info. * Whitespace. * Improve test coverage.

fisx mentioned this pull request Sep 10, 2021

Various improvements and fixes around SAML/SCIM #1735

Merged

10 tasks

fisx added 18 commits September 10, 2021 20:55

Allow two teams with same IdP (entityID).

1c2d9e5

RESET_PLS

2a3d79b

Revert "RESET_PLS"

3b229e9

This reverts commit 17c22336940529a014d649e3d17eed6de6f1c05d.

Tests.

05e269d

...

fc588eb

...

182af83

...

47c377e

...

d2c0467

...

d09283e

RESET_PLS

e4c9097

Revert "RESET_PLS"

da36da6

This reverts commit 8c4316a034f3d248e8eeb4f1060a3ade21c9d467.

...

c9d3a71

...

31ff727

Fix typo.

57d06ae

...

f944be2

RESET_PLS

188626b

Revert "RESET_PLS"

83acf32

This reverts commit e0853d3a4a4a6ddbaf189eeff5c7633f62b016cf.

Try out skipAPIVersions.

edfb77d

fisx force-pushed the various-fixes-2 branch from e0853d3 to edfb77d Compare September 10, 2021 18:56

fisx added 9 commits September 10, 2021 21:00

Update stack.yaml.

a8675c9

Fix stale comment.

3eca40e

better test.

1124443

fix authnreq issuer computation.

24c783f

fix authnreq issuer computation *test*.

23e5e9c

Test multi-team idp issuer (failing).

cf36000

Fix: it's ok to have two teams with the same IdP issuer/entityID...

fe57c6c

... as long as they really live in different teams!

The fix is becoming more invovled...

f0111fc

More error info when looking up idps.

...

42865d3

fisx commented Sep 13, 2021

View reviewed changes

fisx marked this pull request as ready for review September 13, 2021 14:03

fisx changed the title ~~Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team.~~ Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team. [skip ci] Sep 13, 2021

fisx added 4 commits September 13, 2021 16:04

Update services/spar/test-integration/Test/Spar/APISpec.hs

1d15c4e

Update services/spar/test-integration/Test/Spar/APISpec.hs

3950ce6

...

178528f

...

5ede16f

julialongtin approved these changes Sep 13, 2021

View reviewed changes

add a changelog entry.

83f3bc5

jschaul changed the title ~~Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team. [skip ci]~~ Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team. Sep 13, 2021

cassandra-schema docs

a7bab5d

Nit-pick.

91de317

fisx added 7 commits September 13, 2021 22:00

Reduce code duplication.

6f91c57

Fix: test against sso-url from config.

8cfa08b

(not one that we falsely assume will always be configured.)

ormolu

234f95a

...

b9498f4

...

8cad1b2

More explicit error case handling.

79488d5

...

379c56c

fisx changed the title ~~Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team.~~ Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team. [skip ci] Sep 13, 2021

fisx changed the title ~~Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team. [skip ci]~~ Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team. Sep 13, 2021

fisx commented Sep 13, 2021

View reviewed changes

services/spar/src/Spar/API.hs Show resolved Hide resolved

fisx mentioned this pull request Sep 13, 2021

#1755 follow-up #1763

Merged

10 tasks

fisx merged commit c371a14 into develop Sep 13, 2021

fisx deleted the various-fixes-2 branch September 13, 2021 22:30

jschaul mentioned this pull request Sep 13, 2021

Release 2021-09-14 #1764

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team.#1755

Introduce team-based SP entityIDs to allow IdPs with fixed entityID to serve more than one team.#1755
fisx merged 52 commits intodevelopfrom
various-fixes-2

fisx commented Sep 10, 2021 •

edited by jschaul

Loading

Uh oh!

fisx commented Sep 10, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

julialongtin left a comment

Uh oh!

jschaul commented Sep 13, 2021

Uh oh!

fisx commented Sep 13, 2021

Uh oh!

fisx commented Sep 13, 2021

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

fisx commented Sep 10, 2021 • edited by jschaul Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

fisx commented Sep 10, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

julialongtin left a comment

Choose a reason for hiding this comment

Uh oh!

jschaul commented Sep 13, 2021

Uh oh!

fisx commented Sep 13, 2021

Uh oh!

fisx commented Sep 13, 2021

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fisx commented Sep 10, 2021 •

edited by jschaul

Loading