fix(front): pass member type when assigning roles

skipi · skipi · commit a5fb57aa34d5 · 2025-08-13T14:46:12.000+02:00
diff --git a/front/assets/js/people/change_role_dropdown.js b/front/assets/js/people/change_role_dropdown.js
@@ -67,7 +67,8 @@ export var ChangeRoleDropdown = {
     const body = {
       user_id: roleBtn.attributes.user_id.value,
       project_id: InjectedDataByBackend.ProjectId,
-      role_id: roleBtn.attributes.role_id.value
+      role_id: roleBtn.attributes.role_id.value,
+      member_type: roleBtn.attributes.member_type.value
     }
 
     toggleSpinner()
diff --git a/front/assets/js/people/edit_person/index.tsx b/front/assets/js/people/edit_person/index.tsx
@@ -87,7 +87,7 @@ export const Button = () => {
 
     return user.assignRoleUrl
       .call({
-        body: { user_id: user.id, role_id: selectedRole.id },
+        body: { user_id: user.id, role_id: selectedRole.id, member_type: user.memberType },
       })
       .then((resp) => {
         if (resp.error) {
@@ -413,6 +413,8 @@ class User {
   id: string;
   name: string;
   email: string;
+  memberType: string;
+
   roles: UserRole[] = [];
   changeEmailUrl: toolbox.APIRequest.Url<{ email: string, message: string, }>;
   assignRoleUrl: toolbox.APIRequest.Url<{ password: string, message: string, }>;
@@ -426,6 +428,7 @@ class User {
     user.id = json.id as string;
     user.name = json.name as string;
     user.email = json.email as string;
+    user.memberType = json.member_type as string;
     user.roles = json.roles.map((role: any) => {
       return new UserRole({
         id: role.id,
diff --git a/front/lib/front/models/service_account.ex b/front/lib/front/models/service_account.ex
@@ -195,6 +195,13 @@ defmodule Front.Models.ServiceAccount do
   end
 
   defp assign_role(org_id, user_id, service_account, role_id) do
-    RoleManagement.assign_role(user_id, org_id, service_account.id, role_id)
+    RoleManagement.assign_role(
+      user_id,
+      org_id,
+      service_account.id,
+      role_id,
+      "",
+      "service_account"
+    )
   end
 end
diff --git a/front/lib/front/rbac/role_management.ex b/front/lib/front/rbac/role_management.ex
@@ -180,21 +180,48 @@ defmodule Front.RBAC.RoleManagement do
     If the 'project_id' parameter is not passed, it is interpreted as the role being assigned
     within the organization scope.
   """
-  @spec assign_role(id(), id(), id(), id(), String.t()) :: {:ok, String.t()} | {:error, any}
-  def assign_role(requester_id, org_id, subject_id, role_id, project_id \\ "") do
+  @spec assign_role(id(), id(), id(), id(), String.t(), String.t()) ::
+          {:ok, String.t()} | {:error, any}
+  def assign_role(
+        requester_id,
+        org_id,
+        subject_id,
+        role_id,
+        project_id \\ "",
+        subject_type \\ "user"
+      ) do
     Watchman.benchmark("assign_role.duration", fn ->
       Logger.info(
-        "Assigning role: subject_id: #{subject_id}, org_id: #{org_id}, role_id: #{role_id}, project_id: #{project_id}"
+        "Assigning role: subject_id: #{subject_id}, org_id: #{org_id}, role_id: #{role_id}, project_id: #{project_id}, subject_type: #{subject_type}"
       )
 
+      subject_type =
+        case subject_type do
+          "service_account" ->
+            InternalApi.RBAC.SubjectType.value(:SERVICE_ACCOUNT)
+
+          "group" ->
+            InternalApi.RBAC.SubjectType.value(:GROUP)
+
+          # Defaults to user
+          "user" ->
+            InternalApi.RBAC.SubjectType.value(:USER)
+
+          _ ->
+            Logger.warn("Unrecognized subject type: #{subject_type}, defaulting to user")
+            InternalApi.RBAC.SubjectType.value(:USER)
+        end
+
+      subject = RBAC.Subject.new(subject_id: subject_id, type: subject_type)
+
       req =
         RBAC.AssignRoleRequest.new(
           role_assignment:
             RBAC.RoleAssignment.new(
               role_id: role_id,
               org_id: org_id,
               project_id: project_id,
-              subject: RBAC.Subject.new(subject_id: subject_id)
+              subject: subject
             ),
           requester_id: requester_id
         )
diff --git a/front/lib/front_web/controllers/people_controller.ex b/front/lib/front_web/controllers/people_controller.ex
@@ -139,6 +139,7 @@ defmodule FrontWeb.PeopleController do
       user_id = params["user_id"]
       role_id = params["role_id"]
       requester_id = conn.assigns.user_id
+      member_type = params["member_type"] || "user"
 
       conn =
         conn
@@ -148,7 +149,14 @@ defmodule FrontWeb.PeopleController do
       if conn.halted() do
         {:error, :render_404}
       else
-        case RoleManagement.assign_role(requester_id, org_id, user_id, role_id, project_id) do
+        case RoleManagement.assign_role(
+               requester_id,
+               org_id,
+               user_id,
+               role_id,
+               project_id,
+               member_type
+             ) do
           {:ok, _} ->
             log_assign_role(conn, user_id, org_id, role_id, project_id)
 
diff --git a/front/lib/front_web/templates/people/members/__change_role_btn.html.eex b/front/lib/front_web/templates/people/members/__change_role_btn.html.eex
@@ -10,7 +10,7 @@
   <div id="role_selector_<%= @member.id %>" class="dn">
     <%= for role <- @roles do %>
       <%= if role.name != "Owner" || @permissions["organization.change_owner"] do %>
-        <%= construct_role_dropdown_option(role, @member) %>
+        <%= construct_role_dropdown_option(role, @member, @member_type) %>
       <% end %>
     <% end %>
   </div>
diff --git a/front/lib/front_web/templates/people/members/_member.html.eex b/front/lib/front_web/templates/people/members/_member.html.eex
@@ -7,9 +7,9 @@
         <div class="flex items-center">
           <span class="ml1 b">
             <%= cond do %>
-              <% Map.get(assigns, :is_service_account?) -> %>
+              <% @member_type == :service_account -> %>
                 <span class="black"><%= @member.name %></span>
-              <% @is_group? -> %>
+              <% @member_type == :group -> %>
                 <%= if @permissions["organization.people.manage"] do %>
                   <span class="black pointer" style="cursor: pointer;" name="modify-group-btn" group_id="<%= @member.id %>"><%= @member.name %></span>
                 <% else %>
@@ -32,16 +32,16 @@
         <div class="button-group">
           <%= unless !@org_scope? && Front.ce_roles?() do %>
             <%= if Front.ce_roles?() do %>
-              <div class="app-edit-person" data-config="<%= Poison.encode!(edit_person_config(@conn, @member, @roles, @permissions)) %>"></div>
+              <div class="app-edit-person" data-config="<%= Poison.encode!(edit_person_config(@conn, @member, @member_type, @roles, @permissions)) %>"></div>
             <% else %>
-              <%= render "members/__change_role_btn.html", member: @member, roles: @roles, permissions: @permissions %>
+              <%= render "members/__change_role_btn.html", member: @member, member_type: @member_type, roles: @roles, permissions: @permissions %>
             <% end %>
           <% end %>
           <%= cond do %>
-            <% @is_group? and @org_scope? -> %>
+            <% @member_type == :group and @org_scope? -> %>
               <%= render "members/__modify_group_button.html", group: @member, permissions: @permissions %>
               <%= render "members/__delete_group_button.html", group: @member, conn: @conn, permissions: @permissions %>
-            <% Map.get(assigns, :is_service_account?) -> %>
+            <% @member_type == :service_account -> %>
               <%= if @org_scope? || !Front.ce_roles?() || "Member" in member_role_names do %>
                 <%= render "members/__remove_member_btn.html", member: @member %>
               <% end %>
diff --git a/front/lib/front_web/templates/people/members/members_list.html.eex b/front/lib/front_web/templates/people/members/members_list.html.eex
@@ -18,7 +18,7 @@
 
       <div id="groups">
         <%= @groups |> Enum.map(fn (group) -> %>
-          <%= render "members/_member.html", is_group?: true, member: group, roles: @roles, org_scope?: @org_scope?, conn: @conn, permissions: @permissions %>
+          <%= render "members/_member.html", member_type: :group, member: group, roles: @roles, org_scope?: @org_scope?, conn: @conn, permissions: @permissions %>
         <% end) %>
       </div>
     </div>
@@ -39,7 +39,7 @@
 
     <div id="members">
       <%= @members |> Enum.map(fn (member) -> %>
-        <%= render "members/_member.html", is_group?: false, member: member, roles: @roles, org_scope?: @org_scope?, conn: @conn, permissions: @permissions %>
+        <%= render "members/_member.html", member_type: :user, member: member, roles: @roles, org_scope?: @org_scope?, conn: @conn, permissions: @permissions %>
       <% end) %>
     </div>
   </div>
@@ -61,7 +61,7 @@
 
       <div id="service_accounts">
         <%= @service_accounts |> Enum.map(fn (service_account) -> %>
-          <%= render "members/_member.html", is_group?: false, is_service_account?: true, member: service_account, roles: @roles, org_scope?: @org_scope?, conn: @conn, permissions: @permissions %>
+          <%= render "members/_member.html", member_type: :service_account, member: service_account, roles: @roles, org_scope?: @org_scope?, conn: @conn, permissions: @permissions %>
         <% end) %>
         <%= if Enum.empty?(@service_accounts) do %>
           <div class="pv5 tc">
diff --git a/front/lib/front_web/views/people_view.ex b/front/lib/front_web/views/people_view.ex
@@ -97,7 +97,7 @@ defmodule FrontWeb.PeopleView do
     |> Enum.filter(& &1)
   end
 
-  def edit_person_config(conn, member, roles, permissions) do
+  def edit_person_config(conn, member, member_type, roles, permissions) do
     filtered_roles =
       roles
       |> Enum.filter(fn
@@ -111,6 +111,7 @@ defmodule FrontWeb.PeopleView do
         avatar: member.avatar,
         name: member.name,
         email: member.email,
+        member_type: member_type,
         roles: build_roles(member, filtered_roles),
         reset_password_url:
           url(:post, people_path(conn, :reset_password, member.id, format: "json")),
@@ -276,7 +277,7 @@ defmodule FrontWeb.PeopleView do
     end)
   end
 
-  def construct_role_dropdown_option(role, member) do
+  def construct_role_dropdown_option(role, member, member_type) do
     role_selected? = role.name in Enum.map(member.subject_role_bindings, & &1.role.name)
 
     binding_source =
@@ -289,7 +290,7 @@ defmodule FrontWeb.PeopleView do
       end
 
     """
-    <div role_id="#{role.id}" user_id="#{member.id}" name="role_button" class="#{extrapolate_role_div_class(role_selected?, binding_source)}", style="#{extrapolate_role_div_style(binding_source)}">
+    <div role_id="#{role.id}" user_id="#{member.id}" member_type="#{member_type}" name="role_button" class="#{extrapolate_role_div_class(role_selected?, binding_source)}", style="#{extrapolate_role_div_style(binding_source)}">
       <div style="flex-direction: column; display: flex;">
         #{if role_selected?,
       do: '<span class="material-symbols-outlined mr1">done</span>#{git_icon(binding_source)}',

Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,8 @@ export var ChangeRoleDropdown = {`
`67`	`67`	`const body = {`
`68`	`68`	`user_id: roleBtn.attributes.user_id.value,`
`69`	`69`	`project_id: InjectedDataByBackend.ProjectId,`
`70`		`- role_id: roleBtn.attributes.role_id.value`
	`70`	`+ role_id: roleBtn.attributes.role_id.value,`
	`71`	`+ member_type: roleBtn.attributes.member_type.value`
`71`	`72`	`}`
`72`	`73`
`73`	`74`	`toggleSpinner()`