Fix to block requesting the approval to the deployer. (#150)

* Fix to block request the approval to the deployer

* Fix not to display the deployer in candidates of approval

Author: Noah Hanjun Lee

internal/server/api/v1/repos/approval.go

@@ -153,15 +153,21 @@ func (r *Repo) CreateApproval(c *gin.Context) {
 	}
 
 	if _, err := r.i.FindPermOfRepo(ctx, re, user); ent.IsNotFound(err) {
-		r.log.Warn("The permission is not found.", zap.Error(err))
-		gb.ErrorResponse(c, http.StatusUnprocessableEntity, "There is no permssion for the repository.")
+		r.log.Warn("The approver has no permission for the repository.", zap.Error(err))
+		gb.ErrorResponse(c, http.StatusUnprocessableEntity, "The approver has no permission for the repository.")
 		return
 	} else if err != nil {
 		r.log.Error("It has failed to get the perm.", zap.Error(err))
 		gb.ErrorResponse(c, http.StatusInternalServerError, "It has failed to get the perm.")
 		return
 	}
 
+	if d.Edges.User != nil && user.ID == d.Edges.User.ID {
+		r.log.Warn("The deployer can not be the approver.", zap.Error(err))
+		gb.ErrorResponse(c, http.StatusUnprocessableEntity, "The deployer can not be the approver.")
+		return
+	}
+
 	ap, err := r.i.CreateApproval(ctx, &ent.Approval{
 		UserID:       user.ID,
 		DeploymentID: d.ID,
@@ -192,7 +198,7 @@ func (r *Repo) CreateApproval(c *gin.Context) {
 	gb.Response(c, http.StatusCreated, ap)
 }
 
-func (r *Repo) UpdateApproval(c *gin.Context) {
+func (r *Repo) UpdateMyApproval(c *gin.Context) {
 	ctx := c.Request.Context()
 
 	var (

internal/server/api/v1/repos/approval_test.go

@@ -21,6 +21,66 @@ var (
 )
 
 func TestRepo_CreateApproval(t *testing.T) {
+	t.Run("Validate to request the approval to the deployer.", func(t *testing.T) {
+		input := struct {
+			number  int
+			payload *approvalPostPayload
+		}{
+			number: 7,
+			payload: &approvalPostPayload{
+				UserID: 4,
+			},
+		}
+
+		ctrl := gomock.NewController(t)
+		m := mock.NewMockInteractor(ctrl)
+
+		t.Log("MOCK - Find the deployment by ID.")
+		m.
+			EXPECT().
+			FindDeploymentOfRepoByNumber(ctx, any, input.number).
+			Return(&ent.Deployment{
+				ID: input.number,
+				Edges: ent.DeploymentEdges{
+					User: &ent.User{
+						ID: input.payload.UserID,
+					},
+				},
+			}, nil)
+
+		t.Log("MOCK - Find the user, and check the permission.")
+		m.
+			EXPECT().
+			FindUserByID(ctx, input.payload.UserID).
+			Return(&ent.User{
+				ID: input.payload.UserID,
+			}, nil)
+
+		m.
+			EXPECT().
+			FindPermOfRepo(ctx, any, &ent.User{
+				ID: input.payload.UserID,
+			}).
+			Return(&ent.Perm{}, nil)
+
+		gin.SetMode(gin.ReleaseMode)
+
+		r := NewRepo(RepoConfig{}, m)
+		router := gin.New()
+		router.POST("/deployments/:number/approvals", func(c *gin.Context) {
+			c.Set(KeyRepo, &ent.Repo{})
+		}, r.CreateApproval)
+
+		body, _ := json.Marshal(input.payload)
+		req, _ := http.NewRequest("POST", fmt.Sprintf("/deployments/%d/approvals", input.number), bytes.NewBuffer(body))
+
+		w := httptest.NewRecorder()
+		router.ServeHTTP(w, req)
+
+		if w.Code != http.StatusUnprocessableEntity {
+			t.Fatalf("Code != %d, wanted %d", w.Code, http.StatusOK)
+		}
+	})
 
 	t.Run("Create a new approval.", func(t *testing.T) {
 		input := struct {

internal/server/router.go

@@ -141,7 +141,7 @@ func NewRouter(c *RouterConfig) *gin.Engine {
 		repov1.GET("/:namespace/:name/deployments/:number/approvals", rm.RepoReadPerm(), r.ListApprovals)
 		repov1.POST("/:namespace/:name/deployments/:number/approvals", rm.RepoReadPerm(), r.CreateApproval)
 		repov1.GET("/:namespace/:name/deployments/:number/approval", rm.RepoReadPerm(), r.GetMyApproval)
-		repov1.PATCH("/:namespace/:name/deployments/:number/approval", rm.RepoReadPerm(), r.UpdateApproval)
+		repov1.PATCH("/:namespace/:name/deployments/:number/approval", rm.RepoReadPerm(), r.UpdateMyApproval)
 		repov1.GET("/:namespace/:name/approvals/:aid", rm.RepoReadPerm(), r.GetApproval)
 		repov1.DELETE("/:namespace/:name/approvals/:aid", rm.RepoReadPerm(), r.DeleteApproval)
 		repov1.GET("/:namespace/:name/locks", rm.RepoReadPerm(), r.ListLocks)

ui/public/logo192.png

@@ -268,7 +268,7 @@ export const deploymentSlice = createSlice({
                 state.candidates = []
             })
             .addCase(searchCandidates.fulfilled, (state, action) => {
-                state.candidates = action.payload
+                state.candidates = action.payload.filter(candidate => candidate.id !== state.deployment?.deployer?.id)
             })
             .addCase(createApproval.fulfilled, (state, action) => {
                 state.approvals.push(action.payload)

ui/src/redux/deployment.tsx

@@ -30,6 +30,7 @@ import {
     getTag, 
     createDeployment,
     createApproval,
+    getMe,
 } from '../apis'
 
 // fetch all at the first page.
@@ -61,6 +62,7 @@ interface RepoDeployState {
     */
     approvers: User[]
     candidates: User[]
+    user?: User
     deploying: RequestStatus
     deployId: string
 }
@@ -253,6 +255,18 @@ export const searchCandidates = createAsyncThunk<User[], string, { state: {repoD
     }
 )
 
+export const fetchUser = createAsyncThunk<User, void, { state: {repoDeploy: RepoDeployState }}>(
+    "repoDeploy/fetchUser",
+    async (_, { rejectWithValue }) => {
+        try {
+            const user = await getMe()
+            return user
+        } catch(e) {
+            return rejectWithValue(e)
+        }
+    }
+)
+
 export const deploy = createAsyncThunk<void, void, { state: {repoDeploy: RepoDeployState}}> (
     "repoDeploy/deploy",
     async (_ , { getState, rejectWithValue, requestId }) => {
@@ -400,7 +414,10 @@ export const repoDeploySlice = createSlice({
                 state.candidates = []
             })
             .addCase(searchCandidates.fulfilled, (state, action) => {
-                state.candidates = action.payload
+                state.candidates = action.payload.filter(candidate => (candidate.id !== state.user?.id))
+            })
+            .addCase(fetchUser.fulfilled, (state, action) => {
+                state.user = action.payload
             })
             .addCase(deploy.pending, (state, action) => {
                 if (state.deploying === RequestStatus.Idle) {

ui/src/redux/repoDeploy.tsx

@@ -6,7 +6,8 @@ import {
     listDeployments, 
     rollbackDeployment,
     createApproval,
-    listPerms
+    listPerms,
+    getMe
 } from "../apis"
 import { 
     User,
@@ -39,6 +40,7 @@ interface RepoRollbackState {
     */
     approvers: User[]
     candidates: User[]
+    user?: User
     deployId: string
     deploying: RequestStatus
 }
@@ -100,6 +102,18 @@ export const searchCandidates = createAsyncThunk<User[], string, { state: {repoR
     }
 )
 
+export const fetchUser = createAsyncThunk<User, void, { state: {repoRollback: RepoRollbackState }}>(
+    "repoRollback/fetchUser",
+    async (_, { rejectWithValue }) => {
+        try {
+            const user = await getMe()
+            return user
+        } catch(e) {
+            return rejectWithValue(e)
+        }
+    }
+)
+
 export const rollback = createAsyncThunk<void, void, { state: {repoRollback: RepoRollbackState}}> (
     "repoRollback/deploy",
     async (_ , { getState, rejectWithValue, requestId }) => {
@@ -196,7 +210,10 @@ export const repoRollbackSlice = createSlice({
                 state.candidates = []
             })
             .addCase(searchCandidates.fulfilled, (state, action) => {
-                state.candidates = action.payload
+                state.candidates = action.payload.filter(candidate => (candidate.id !== state.user?.id))
+            })
+            .addCase(fetchUser.fulfilled, (state, action) => {
+                state.user = action.payload
             })
             .addCase(rollback.pending, (state, action) => {
                 if (state.deploying === RequestStatus.Idle) {

ui/src/redux/repoRollback.tsx

@@ -56,10 +56,11 @@ export default function DeploymentView(): JSX.Element {
         const f = async () => {
             await dispatch(slice.actions.init({namespace, name, number: parseInt(number, 10)}))
             await dispatch(fetchDeployment())
-            await dispatch(slice.actions.setDisplay(true))
-            await dispatch(fetchDeploymentChanges())
             await dispatch(fetchApprovals())
             await dispatch(fetchMyApproval())
+            await dispatch(slice.actions.setDisplay(true))
+            await dispatch(fetchDeploymentChanges())
+            await dispatch(searchCandidates(""))
         }
         f()
 

ui/src/views/Deployment.tsx

@@ -19,6 +19,7 @@ import {
     checkTag,
     addTagManually, 
     searchCandidates,
+    fetchUser,
     deploy} from "../redux/repoDeploy"
 
 import DeployForm, {Option} from "../components/DeployForm"
@@ -55,6 +56,7 @@ export default function RepoDeploy(): JSX.Element {
             await dispatch(actions.setDisplay(true))
             await dispatch(fetchBranches())
             await dispatch(fetchTags())
+            await dispatch(fetchUser())
             await dispatch(searchCandidates(""))
         }
         f()

ui/src/views/RepoDeploy.tsx

@@ -4,7 +4,14 @@ import { PageHeader, Result, Button } from 'antd'
 import { shallowEqual } from "react-redux";
 
 import { useAppDispatch, useAppSelector } from "../redux/hooks"
-import { repoRollbackSlice, fetchConfig, fetchDeployments, searchCandidates, rollback } from "../redux/repoRollback"
+import { 
+    repoRollbackSlice, 
+    fetchConfig, 
+    fetchDeployments, 
+    searchCandidates, 
+    fetchUser,
+    rollback,
+} from "../redux/repoRollback"
 
 import { User, Deployment, RequestStatus, Env } from '../models'
 import RollbackForm from "../components/RollbackForm";
@@ -33,6 +40,7 @@ export default function RepoHome(): JSX.Element {
             await dispatch(actions.init({namespace, name}))
             await dispatch(fetchConfig())
             await dispatch(actions.setDisplay(true))
+            await dispatch(fetchUser())
             await dispatch(searchCandidates(""))
         }
         f()