Merge pull request concourse#6021 from concourse/issue/5939

fly: admins can login to any team that exists

atc/api/config_test.go

@@ -981,6 +981,39 @@ jobs:
 							})
 						})
 					})
+
+					Context("there is a problem fetching the team", func() {
+						BeforeEach(func() {
+							request.Header.Set("Content-Type", "application/json")
+
+							payload, err := json.Marshal(pipelineConfig)
+							Expect(err).NotTo(HaveOccurred())
+
+							request.Body = gbytes.BufferWithBytes(payload)
+						})
+
+						Context("when the team is not found", func() {
+							BeforeEach(func() {
+								dbTeamFactory.FindTeamReturns(nil, false, nil)
+							})
+
+							It("returns 404", func() {
+								Expect(response.StatusCode).To(Equal(http.StatusNotFound))
+							})
+						})
+
+						Context("when finding the team fails", func() {
+							BeforeEach(func() {
+								dbTeamFactory.FindTeamReturns(nil, false, errors.New("failed"))
+							})
+
+							It("returns 500", func() {
+								Expect(response.StatusCode).To(Equal(http.StatusInternalServerError))
+							})
+						})
+					})
+
+
 				})
 
 				Context("when the Content-Type is unsupported", func() {

atc/api/configserver/save.go

@@ -101,7 +101,7 @@ func (s *Server) SaveConfig(w http.ResponseWriter, r *http.Request) {
 
 	if !found {
 		session.Debug("team-not-found")
-		w.WriteHeader(http.StatusInternalServerError)
+		w.WriteHeader(http.StatusNotFound)
 		return
 	}
 

fly/commands/login.go

@@ -150,36 +150,14 @@ func (command *LoginCommand) Execute(args []string) error {
 
 	fmt.Println("")
 
-	verifyTarget, err := rc.NewAuthenticatedTarget("verify",
-		client.URL(),
-		command.TeamName,
-		command.Insecure,
-		&rc.TargetToken{
-			Type:  tokenType,
-			Value: tokenValue,
-		},
-		target.CACert(),
-		false)
-	if err != nil {
-		return err
-	}
-
-	userInfo, err := verifyTarget.Client().UserInfo()
+	err = command.verifyTeamExists(client.URL(), rc.TargetToken{
+		Type:  tokenType,
+		Value: tokenValue,
+	}, target.CACert())
 	if err != nil {
 		return err
 	}
 
-	if !userInfo.IsAdmin {
-		if userInfo.Teams != nil {
-			_, ok := userInfo.Teams[command.TeamName]
-			if !ok {
-				return errors.New("you are not a member of '" + command.TeamName + "' or the team does not exist")
-			}
-		} else {
-			return errors.New("unable to verify role on team")
-		}
-	}
-
 	return command.saveTarget(
 		client.URL(),
 		&rc.TargetToken{
@@ -526,3 +504,49 @@ func (command *LoginCommand) legacyAuth(target rc.Target, browserOnly bool, isRa
 
 	return "", "", nil
 }
+
+func (command *LoginCommand) verifyTeamExists(clientUrl string, token rc.TargetToken, caCert string) error {
+	verifyTarget, err := rc.NewAuthenticatedTarget("verify",
+		clientUrl,
+		command.TeamName,
+		command.Insecure,
+		&token,
+		caCert,
+		false)
+	if err != nil {
+		return err
+	}
+
+	userInfo, err := verifyTarget.Client().UserInfo()
+	if err != nil {
+		return err
+	}
+
+	if !userInfo.IsAdmin {
+		if userInfo.Teams != nil {
+			_, ok := userInfo.Teams[command.TeamName]
+			if !ok {
+				return errors.New("you are not a member of '" + command.TeamName + "' or the team does not exist")
+			}
+		} else {
+			return errors.New("unable to verify role on team")
+		}
+	} else {
+		teams, err := verifyTarget.Client().ListTeams()
+		if err != nil {
+			return err
+		}
+		var found bool
+		for _, team := range teams {
+			if team.Name == command.TeamName {
+				found = true
+				break
+			}
+		}
+		if !found {
+			return errors.New("team '" + command.TeamName + "' does not exist")
+		}
+	}
+
+	return nil
+}

fly/integration/login_test.go

@@ -709,7 +709,7 @@ var _ = Describe("login Command", func() {
 		})
 
 		Context("when logging in as an admin user", func() {
-			It("can login to any team", func() {
+			It("can login to any team that exists", func() {
 				loginATCServer.AppendHandlers(
 					infoHandler(),
 					tokenHandler(),
@@ -720,6 +720,21 @@ var _ = Describe("login Command", func() {
 							"is_admin":  true,
 						}),
 					),
+					ghttp.CombineHandlers(
+						ghttp.VerifyRequest("GET", "/api/v1/teams"),
+						ghttp.RespondWithJSONEncoded(200, []atc.Team{
+							{
+								ID:   1,
+								Name: "any-team",
+								Auth: atc.TeamAuth{
+									"owner": map[string][]string{
+										"groups": []string{},
+										"users":  []string{},
+									},
+								},
+							},
+						}),
+					),
 				)
 
 				flyCmd := exec.Command(flyPath, "-t", "some-target", "login", "-c", loginATCServer.URL(), "-n", "any-team", "-u", "admin_user", "-p", "pass")
@@ -732,6 +747,44 @@ var _ = Describe("login Command", func() {
 				<-sess.Exited
 				Expect(sess.ExitCode()).To(Equal(0))
 			})
+			It("fails to login if the team does not exist", func() {
+				loginATCServer.AppendHandlers(
+					infoHandler(),
+					tokenHandler(),
+					ghttp.CombineHandlers(
+						ghttp.VerifyRequest("GET", "/api/v1/user"),
+						ghttp.RespondWithJSONEncoded(200, map[string]interface{}{
+							"user_name": "admin_user",
+							"is_admin":  true,
+						}),
+					),
+					ghttp.CombineHandlers(
+						ghttp.VerifyRequest("GET", "/api/v1/teams"),
+						ghttp.RespondWithJSONEncoded(200, []atc.Team{
+							{
+								ID:   1,
+								Name: "main",
+								Auth: atc.TeamAuth{
+									"owner": map[string][]string{
+										"groups": []string{},
+										"users":  []string{},
+									},
+								},
+							},
+						}),
+					),
+				)
+
+				flyCmd := exec.Command(flyPath, "-t", "some-target", "login", "-c", loginATCServer.URL(), "-n", "doesNotExist", "-u", "admin_user", "-p", "pass")
+
+				sess, err := gexec.Start(flyCmd, GinkgoWriter, GinkgoWriter)
+				Expect(err).NotTo(HaveOccurred())
+
+				Eventually(sess.Err).Should(gbytes.Say("error: team 'doesNotExist' does not exist"))
+
+				<-sess.Exited
+				Expect(sess.ExitCode()).To(Equal(1))
+			})
 		})
 	})
 })