From 78e9c0d2d961bd84a91733c346181a98ee51e6fe Mon Sep 17 00:00:00 2001
From: John Cowen <johncowen@users.noreply.github.com>
Date: Tue, 11 Jan 2022 11:02:46 +0000
Subject: [PATCH] ui: Ensure the partition is passed through to the request for
 the SSO auth URL (#11979)

* Make sure the mocks reflect the requested partition/namespace

* Ensure partition is passed through to the HTTP adapter

* Pass AuthMethod object through to TokenSource in order to use Partition

* Change up docs and add potential improvements for future

* Pass the query partition back onto the response

* Make sure the OIDC callback mock returns a Partition

* Enable OIDC provider mock overwriting during acceptance testing

* Make sure we can enable partitions and SSO post bootup only required

...for now

* Wire up oidc provider mocking

* Add SSO full auth flow acceptance tests
---
 .changelog/11979.txt                          |  4 +++
 .../app/components/auth-form/index.hbs        |  2 +-
 .../app/components/token-source/README.mdx    | 11 ++++++-
 .../app/components/token-source/index.hbs     |  8 ++---
 .../app/serializers/oidc-provider.js          |  1 +
 .../app/services/repository/oidc-provider.js  |  3 +-
 .../consul-ui/mock-api/v1/acl/oidc/callback   |  5 ++++
 .../mock-api/v1/internal/ui/oidc-auth-methods |  4 +--
 .../consul-ui/tests/acceptance/login.feature  | 30 +++++++++++++++++++
 .../consul-ui/tests/helpers/set-cookies.js    |  3 ++
 .../consul-ui/tests/helpers/type-to-url.js    |  3 ++
 ui/packages/consul-ui/tests/steps.js          |  9 +++++-
 .../consul-ui/tests/steps/doubles/http.js     |  5 +++-
 .../consul-ui/tests/steps/doubles/model.js    |  8 +++++
 14 files changed, 85 insertions(+), 11 deletions(-)
 create mode 100644 .changelog/11979.txt

diff --git a/.changelog/11979.txt b/.changelog/11979.txt
new file mode 100644
index 0000000000..8ef0e855b1
--- /dev/null
+++ b/.changelog/11979.txt
@@ -0,0 +1,4 @@
+```release-note:bug
+ui: Ensure partition query parameter is passed through to all OIDC related API
+requests
+```
diff --git a/ui/packages/consul-ui/app/components/auth-form/index.hbs b/ui/packages/consul-ui/app/components/auth-form/index.hbs
index 3578e467d1..b9ba008f99 100644
--- a/ui/packages/consul-ui/app/components/auth-form/index.hbs
+++ b/ui/packages/consul-ui/app/components/auth-form/index.hbs
@@ -154,7 +154,7 @@ as |TabState IgnoredGuard IgnoredAction tabDispatch tabState|>
       @nspace={{or this.value.Namespace @nspace}}
       @partition={{or this.value.Partition @partition}}
       @type={{if this.value.Name 'oidc' 'secret'}}
-      @value={{if this.value.Name this.value.Name this.value}}
+      @value={{this.value}}
       @onchange={{queue (action dispatch "RESET") @onsubmit}}
       @onerror={{queue (action (mut this.error) value="error.errors.firstObject") (action dispatch "ERROR")}}
     />
diff --git a/ui/packages/consul-ui/app/components/token-source/README.mdx b/ui/packages/consul-ui/app/components/token-source/README.mdx
index 8a451f2c83..186cba5d4f 100644
--- a/ui/packages/consul-ui/app/components/token-source/README.mdx
+++ b/ui/packages/consul-ui/app/components/token-source/README.mdx
@@ -21,6 +21,15 @@ This component **does not store the resulting token**, it only emits it via
 its `onchange` argument/event handler. Errors are emitted via the `onerror`
 argument/event handler.
 
+## Potential improvements
+
+We could decide to remove the `@type` argument and always require an object
+passed to `@value` instead of a `String|Object`. Alternatively we could still
+allow `String|Object`. Then inside the component we could decide whether to
+use the Consul or SSO depending on the shape of the `@value` argument. All in
+all this means we can remove the `@type` argument making a slimmer component
+API.
+
 ```hbs preview-template
 <figure>
   <figcaption>Provide a widget to login with</figcaption>
@@ -75,7 +84,7 @@ argument/event handler.
 | `nspace` | `String` | | The name of the current namespace |
 | `partition` | `String` | | The name of the current partition |
 | `type` | `String` | | `secret` or `oidc`. `secret` is just traditional login, whereas `oidc` uses the users OIDC provider |
-| `value` | `String` | | When `type` is `secret` this should be the users secret. When `type` is `oidc` this should be the name of the `AuthMethod` to use for authentication |
+| `value` | `String|Object` | | When `type` is `secret` this should be the users secret. When `type` is `oidc` this should be object returned by Consul's AuthMethod HTTP API endpoint |
 | `onchange` | `Function` |  | The action to fire when the data changes. Emits an Event-like object with a `data` property containing the jwt data, in this case the autorizationCode and the status |
 | `onerror` | `Function` |  | The action to fire when an error occurs. Emits ErrorEvent object with an `error` property containing the Error. |
 
diff --git a/ui/packages/consul-ui/app/components/token-source/index.hbs b/ui/packages/consul-ui/app/components/token-source/index.hbs
index 94186be1ba..962fc7774a 100644
--- a/ui/packages/consul-ui/app/components/token-source/index.hbs
+++ b/ui/packages/consul-ui/app/components/token-source/index.hbs
@@ -7,10 +7,10 @@ as |State Guard Action dispatch state|>
     @cond={{this.isSecret}}
   />
   {{#let
-    (uri '/${partition}/{$nspace}/${dc}'
+    (uri '/${partition}/${nspace}/${dc}'
       (hash
-        partition=@partition
-        nspace=@nspace
+        partition=(or @value.Partition @partition)
+        nspace=(or @value.Namespace @nspace)
         dc=@dc
       )
     )
@@ -30,7 +30,7 @@ as |State Guard Action dispatch state|>
       <DataSource
         @src={{uri (concat path '/oidc/provider/${value}')
           (hash
-            value=@value
+            value=@value.Name
           )
         }}
         @onchange={{queue (action (mut this.provider) value="data") (action dispatch "SUCCESS")}}
diff --git a/ui/packages/consul-ui/app/serializers/oidc-provider.js b/ui/packages/consul-ui/app/serializers/oidc-provider.js
index 1da745dcdf..996b8c7ea6 100644
--- a/ui/packages/consul-ui/app/serializers/oidc-provider.js
+++ b/ui/packages/consul-ui/app/serializers/oidc-provider.js
@@ -22,6 +22,7 @@ export default class OidcSerializer extends Serializer {
           cb(headers, {
             Name: query.id,
             Namespace: query.ns,
+            Partition: query.partition,
             ...body,
           })
         ),
diff --git a/ui/packages/consul-ui/app/services/repository/oidc-provider.js b/ui/packages/consul-ui/app/services/repository/oidc-provider.js
index b1069024e7..2fbd1be319 100644
--- a/ui/packages/consul-ui/app/services/repository/oidc-provider.js
+++ b/ui/packages/consul-ui/app/services/repository/oidc-provider.js
@@ -40,7 +40,7 @@ export default class OidcProviderService extends RepositoryService {
     // with an empty `ns=` Consul will use the namespace that is assigned to
     // the token, and when we get the response we can pick that back off the
     // responses `Namespace` property. As we don't receive a `Namespace`
-    // property here, we have to figure this out ourselves. Biut we also want
+    // property here, we have to figure this out ourselves. But we also want
     // to make this completely invisible to 'the application engineer/a
     // template engineer'. This feels like the best place/way to do it as we
     // are already in a asynchronous method, and we avoid adding extra 'just
@@ -54,6 +54,7 @@ export default class OidcProviderService extends RepositoryService {
     const token = (await this.settings.findBySlug('token')) || {};
     return super.findBySlug({
       ns: params.ns || token.Namespace || 'default',
+      partition: params.partition || token.Partition || 'default',
       dc: params.dc,
       id: params.id,
     });
diff --git a/ui/packages/consul-ui/mock-api/v1/acl/oidc/callback b/ui/packages/consul-ui/mock-api/v1/acl/oidc/callback
index 7aedf0ba07..bcd2819ad3 100644
--- a/ui/packages/consul-ui/mock-api/v1/acl/oidc/callback
+++ b/ui/packages/consul-ui/mock-api/v1/acl/oidc/callback
@@ -5,6 +5,11 @@
     typeof location.search.ns !== 'undefined' ? location.search.ns :
       typeof http.body.Namespace !== 'undefined' ? http.body.Namespace : 'default'
   }",
+  "Partition": "${
+    typeof location.search.partition !== 'undefined' ?
+    location.search.partition :
+      typeof http.body.Partition !== 'undefined' ? http.body.Partition : 'default'
+  }",
   "Local": false,
   "Description": "AuthMethod: ${http.body.AuthMethod}; Code: ${http.body.Code}; State: ${http.body.State}; - ${fake.lorem.sentence()}",
   "Policies": [
diff --git a/ui/packages/consul-ui/mock-api/v1/internal/ui/oidc-auth-methods b/ui/packages/consul-ui/mock-api/v1/internal/ui/oidc-auth-methods
index 068325d103..e8d0a723dd 100644
--- a/ui/packages/consul-ui/mock-api/v1/internal/ui/oidc-auth-methods
+++ b/ui/packages/consul-ui/mock-api/v1/internal/ui/oidc-auth-methods
@@ -16,8 +16,8 @@ return `
     "Name": "${name.split(' ').join('-').toLowerCase()}",
     "DisplayName": "${name}",
     "Kind": "${fake.helpers.randomize(['no-icon', 'google', 'okta', 'auth0', 'microsoft'])}",
-    "Namespace": "default",
-    "Partition": "default"
+    "Namespace": "${typeof location.search.ns !== 'undefined' ? location.search.ns : 'default'}",
+    "Partition": "${typeof location.search.partition !== 'undefined' ? location.search.partition : 'default'}"
   }
 `})
   }
diff --git a/ui/packages/consul-ui/tests/acceptance/login.feature b/ui/packages/consul-ui/tests/acceptance/login.feature
index 42aff3a1fe..17a4ab82b5 100644
--- a/ui/packages/consul-ui/tests/acceptance/login.feature
+++ b/ui/packages/consul-ui/tests/acceptance/login.feature
@@ -19,3 +19,33 @@ Feature: login
     headers:
       X-Consul-Token: something
     ---
+  @onlyNamespaceable
+  Scenario: Logging in via SSO
+    Given 1 datacenter model with the value "dc-1"
+    And SSO is enabled
+    And partitions are enabled
+    And 1 oidcProvider model from yaml
+    ---
+    - DisplayName: Okta
+      Name: okta
+      Kind: okta
+    ---
+    When I visit the services page for yaml
+    ---
+    dc: dc-1
+    ---
+    And the "okta" oidcProvider responds with from yaml
+    ---
+    state: state-123456789/abcdefghijklmnopqrstuvwxyz
+    code: code-abcdefghijklmnopqrstuvwxyz/123456789
+    ---
+    And I click login on the navigation
+    And I click "[data-test-tab=tab_sso] button"
+    And I type "partition" into "[name=partition]"
+    And I click ".oidc-select button"
+    Then a GET request was made to "/v1/internal/ui/oidc-auth-methods?dc=dc-1&ns=@namespace&partition=partition"
+    And I click ".okta-oidc-provider"
+    Then a POST request was made to "/v1/acl/oidc/auth-url?dc=dc-1&ns=@!namespace&partition=partition"
+    And a POST request was made to "/v1/acl/oidc/callback?dc=dc-1&ns=@!namespace&partition=partition"
+    And "[data-notification]" has the "notification-authorize" class
+    And "[data-notification]" has the "success" class
diff --git a/ui/packages/consul-ui/tests/helpers/set-cookies.js b/ui/packages/consul-ui/tests/helpers/set-cookies.js
index b3805cb600..053bd90907 100644
--- a/ui/packages/consul-ui/tests/helpers/set-cookies.js
+++ b/ui/packages/consul-ui/tests/helpers/set-cookies.js
@@ -43,6 +43,9 @@ export default function(type, value, doc = document) {
       case 'authMethod':
         key = 'CONSUL_AUTH_METHOD_COUNT';
         break;
+      case 'oidcProvider':
+        key = 'CONSUL_OIDC_PROVIDER_COUNT';
+        break;
       case 'nspace':
         key = 'CONSUL_NSPACE_COUNT';
         break;
diff --git a/ui/packages/consul-ui/tests/helpers/type-to-url.js b/ui/packages/consul-ui/tests/helpers/type-to-url.js
index d35cce0c9b..1e88556ad1 100644
--- a/ui/packages/consul-ui/tests/helpers/type-to-url.js
+++ b/ui/packages/consul-ui/tests/helpers/type-to-url.js
@@ -40,6 +40,9 @@ export default function(type) {
     case 'authMethod':
       requests = ['/v1/acl/auth-methods', '/v1/acl/auth-method/'];
       break;
+    case 'oidcProvider':
+      requests = ['/v1/internal/ui/oidc-auth-methods'];
+      break;
     case 'nspace':
       requests = ['/v1/namespaces', '/v1/namespace/'];
       break;
diff --git a/ui/packages/consul-ui/tests/steps.js b/ui/packages/consul-ui/tests/steps.js
index 74bbf2b199..43b1c0c553 100644
--- a/ui/packages/consul-ui/tests/steps.js
+++ b/ui/packages/consul-ui/tests/steps.js
@@ -103,9 +103,16 @@ export default function({
     let location = context.owner.lookup(`location:${locationType}`);
     return location.getURLFrom();
   };
+  const oidcProvider = function(name, response) {
+    const context = helpers.getContext();
+    const provider = context.owner.lookup('torii-provider:oidc-with-url');
+    provider.popup.open = async function() {
+      return response;
+    };
+  };
 
   models(library, create, setCookie);
-  http(library, respondWith, setCookie);
+  http(library, respondWith, setCookie, oidcProvider);
   visit(library, pages, utils.setCurrentPage, reset);
   click(library, utils.find, helpers.click);
   form(library, utils.find, helpers.fillIn, helpers.triggerKeyEvent, utils.getCurrentPage);
diff --git a/ui/packages/consul-ui/tests/steps/doubles/http.js b/ui/packages/consul-ui/tests/steps/doubles/http.js
index 798fd1670d..b92ec8ae7d 100644
--- a/ui/packages/consul-ui/tests/steps/doubles/http.js
+++ b/ui/packages/consul-ui/tests/steps/doubles/http.js
@@ -1,4 +1,4 @@
-export default function(scenario, respondWith, set) {
+export default function(scenario, respondWith, set, oidc) {
   // respondWith should set the url to return a certain response shape
   scenario
     .given(['the url "$endpoint" responds with a $status status'], function(url, status) {
@@ -12,6 +12,9 @@ export default function(scenario, respondWith, set) {
       }
       respondWith(url, data);
     })
+    .given(['the "$provider" oidcProvider responds with from yaml\n$yaml'], function(name, data) {
+      oidc(name, data);
+    })
     .given('a network latency of $number', function(number) {
       set('CONSUL_LATENCY', number);
     });
diff --git a/ui/packages/consul-ui/tests/steps/doubles/model.js b/ui/packages/consul-ui/tests/steps/doubles/model.js
index 65b11a114a..9a0536266e 100644
--- a/ui/packages/consul-ui/tests/steps/doubles/model.js
+++ b/ui/packages/consul-ui/tests/steps/doubles/model.js
@@ -38,6 +38,14 @@ export default function(scenario, create, set, win = window, doc = document) {
     .given(['ACLs are disabled'], function() {
       doc.cookie = `CONSUL_ACLS_ENABLE=0`;
     })
+    .given(['SSO is enabled'], function() {
+      doc.cookie = `CONSUL_SSO_ENABLE=1`;
+      set('CONSUL_SSO_ENABLE', 1);
+    })
+    .given(['partitions are enabled'], function() {
+      doc.cookie = `CONSUL_PARTITIONS_ENABLE=1`;
+      set('CONSUL_PARTITIONS_ENABLE', 1);
+    })
     .given(['the default ACL policy is "$policy"'], function(policy) {
       set('CONSUL_ACL_POLICY', policy);
     })