Merge branch 'main' from arvados-workbench2.git

[arvados.git] / lib / controller / localdb / login.go

diff --git a/lib/controller/localdb/login.go b/lib/controller/localdb/login.go
index 041336bcaca442e69ea6a23b57d116a82551eaf6..f9b968a705255408132bb6449885c33356728cd2 100644 (file)
--- a/lib/controller/localdb/login.go
+++ b/lib/controller/localdb/login.go
@@ -164,6 +164,8 @@ func (conn *Conn) CreateAPIClientAuthorization(ctx context.Context, rootToken st
        return
 }
 
+var errUserinfoInRedirectTarget = errors.New("redirect target rejected because it contains userinfo")
+
 func validateLoginRedirectTarget(cluster *arvados.Cluster, returnTo string) error {
        u, err := url.Parse(returnTo)
        if err != nil {
@@ -173,9 +175,22 @@ func validateLoginRedirectTarget(cluster *arvados.Cluster, returnTo string) erro
        if err != nil {
                return err
        }
+       if u.User != nil {
+               return errUserinfoInRedirectTarget
+       }
        target := origin(*u)
        for trusted := range cluster.Login.TrustedClients {
-               if origin(url.URL(trusted)) == target {
+               trustedOrigin := origin(url.URL(trusted))
+               if trustedOrigin == target {
+                       return nil
+               }
+               // If TrustedClients has https://*.bar.example, we
+               // trust https://foo.bar.example. Note origin() has
+               // already stripped the incoming Path, so we won't
+               // accidentally trust
+               // https://attacker.example/pwn.bar.example here. See
+               // tests.
+               if strings.HasPrefix(trustedOrigin, u.Scheme+"://*.") && strings.HasSuffix(target, trustedOrigin[len(u.Scheme)+4:]) {
                        return nil
                }
        }