16171: Change issuer config to string to avoid trailing-slash pain.
authorTom Clegg <tom@tomclegg.ca>
Thu, 4 Jun 2020 18:17:28 +0000 (14:17 -0400)
committerTom Clegg <tom@tomclegg.ca>
Thu, 4 Jun 2020 18:17:28 +0000 (14:17 -0400)
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

lib/config/config.default.yml
lib/config/generated_config.go
lib/controller/localdb/login.go
lib/controller/localdb/login_oidc_test.go
sdk/go/arvados/config.go

index be0123574f02139b1bc1f9b920745c0660a3f6c6..ff73c6f469cbb1c24c09ceb07f8b9a5b644d1ad8 100644 (file)
@@ -560,14 +560,9 @@ Clusters:
         # This must be exactly equal to the URL returned by the issuer
         # itself in its config response ("isser" key). If the
         # configured value is "https://example" and the provider
-        # returns "https://example:443" then login will fail, even
-        # though those URLs are equivalent (RFC3986).
-        #
-        # If the configured URL's path component is just "/" then it
-        # is stripped. Therefore, an issuer advertising itself as
-        # "https://example/" cannot be used -- but "https://example",
-        # "https://example/foo", and "https://example/foo/" are
-        # supported.
+        # returns "https://example:443" or "https://example/" then
+        # login will fail, even though those URLs are equivalent
+        # (RFC3986).
         Issuer: ""
 
         # Your client ID and client secret (supplied by the provider).
index 49d86c77f2b010d15b11999d06a54da105c1da52..3a317af88ed81f27130f75626be7fe06773c9058 100644 (file)
@@ -566,14 +566,9 @@ Clusters:
         # This must be exactly equal to the URL returned by the issuer
         # itself in its config response ("isser" key). If the
         # configured value is "https://example" and the provider
-        # returns "https://example:443" then login will fail, even
-        # though those URLs are equivalent (RFC3986).
-        #
-        # If the configured URL's path component is just "/" then it
-        # is stripped. Therefore, an issuer advertising itself as
-        # "https://example/" cannot be used -- but "https://example",
-        # "https://example/foo", and "https://example/foo/" are
-        # supported.
+        # returns "https://example:443" or "https://example/" then
+        # login will fail, even though those URLs are equivalent
+        # (RFC3986).
         Issuer: ""
 
         # Your client ID and client secret (supplied by the provider).
index 04fb82bd5b9eaa09e5007a162e43c8b2dbd0ea95..9a0ee746e64006d08ab2d87981f6f47bf8fbcfa6 100644 (file)
@@ -39,30 +39,10 @@ func chooseLoginController(cluster *arvados.Cluster, railsProxy *railsProxy) log
                        UseGooglePeopleAPI: cluster.Login.Google.AlternateEmailAddresses,
                }
        case !wantGoogle && wantOpenIDConnect && !wantSSO && !wantPAM && !wantLDAP:
-               issuer := cluster.Login.OpenIDConnect.Issuer
-               if issuer.Path == "/" {
-                       // The OIDC library returns an error if the
-                       // config says "https://example/" and the
-                       // issuer identifies itself as
-                       // "https://example", even though those URLs
-                       // are equivalent
-                       // (https://tools.ietf.org/html/rfc3986#section-6.2.3).
-                       //
-                       // Our config loader adds "/" to URLs with
-                       // empty path, so we strip it off here and
-                       // count on the issuer to do the same when
-                       // identifying itself, as Google does.
-                       //
-                       // (Non-empty paths as in
-                       // "https://example/foo/" are preserved by the
-                       // config loader so the config just has to
-                       // match the issuer's response.)
-                       issuer.Path = ""
-               }
                return &oidcLoginController{
                        Cluster:      cluster,
                        RailsProxy:   railsProxy,
-                       Issuer:       issuer.String(),
+                       Issuer:       cluster.Login.OpenIDConnect.Issuer,
                        ClientID:     cluster.Login.OpenIDConnect.ClientID,
                        ClientSecret: cluster.Login.OpenIDConnect.ClientSecret,
                }
index b6e58c9483b3cf0fd28a71b345f86380094576ab..aa437218ff79eaae26ee93d8450af72561387919 100644 (file)
@@ -263,12 +263,12 @@ func (s *OIDCLoginSuite) TestGoogleLogin_PeopleAPIDisabled(c *check.C) {
 func (s *OIDCLoginSuite) TestConfig(c *check.C) {
        s.cluster.Login.Google.Enable = false
        s.cluster.Login.OpenIDConnect.Enable = true
-       s.cluster.Login.OpenIDConnect.Issuer = arvados.URL{Scheme: "https", Host: "accounts.example.com", Path: "/"}
+       s.cluster.Login.OpenIDConnect.Issuer = "https://accounts.example.com/"
        s.cluster.Login.OpenIDConnect.ClientID = "oidc-client-id"
        s.cluster.Login.OpenIDConnect.ClientSecret = "oidc-client-secret"
        localdb := NewConn(s.cluster)
        ctrl := localdb.loginController.(*oidcLoginController)
-       c.Check(ctrl.Issuer, check.Equals, "https://accounts.example.com")
+       c.Check(ctrl.Issuer, check.Equals, "https://accounts.example.com/")
        c.Check(ctrl.ClientID, check.Equals, "oidc-client-id")
        c.Check(ctrl.ClientSecret, check.Equals, "oidc-client-secret")
        c.Check(ctrl.UseGooglePeopleAPI, check.Equals, false)
index 0558de808a354203e9865c164b5a32c619978a6e..dbd9f71099619203bb38f4dd1118b865f5c2f662 100644 (file)
@@ -158,7 +158,7 @@ type Cluster struct {
                }
                OpenIDConnect struct {
                        Enable       bool
-                       Issuer       URL
+                       Issuer       string
                        ClientID     string
                        ClientSecret string
                }