Skip to content

Commit 4a2d99c

Browse files
authored
28398 - Team Admin receives 403 Forbidden when attempting to reset OTP for another team member (#3493)
1 parent 184f853 commit 4a2d99c

File tree

11 files changed

+22
-26
lines changed

11 files changed

+22
-26
lines changed

auth-api/src/auth_api/resources/v1/user.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ def post_user():
135135
return response, status
136136

137137

138-
@bp.route("/<path:username>/otp", methods=["DELETE", "OPTIONS"])
138+
@bp.route("/<path:username>/otp/<int:org_id>", methods=["DELETE", "OPTIONS"])
139139
@cross_origin(origins="*", methods=["DELETE"])
140140
@_jwt.has_one_of_roles(
141141
[
@@ -145,7 +145,7 @@ def post_user():
145145
Role.MANAGE_RESET_OTP.value,
146146
]
147147
)
148-
def delete_user_otp(username):
148+
def delete_user_otp(username, org_id):
149149
"""Delete/Reset the OTP of user profile associated with the provided username."""
150150
try:
151151
user = UserService.find_by_username(username)
@@ -155,7 +155,7 @@ def delete_user_otp(username):
155155
response, status = {"Only BCEID users has OTP", HTTPStatus.BAD_REQUEST}
156156
else:
157157
origin_url = request.environ.get("HTTP_ORIGIN", "localhost")
158-
UserService.delete_otp_for_user(username, origin_url=origin_url)
158+
UserService.delete_otp_for_user(user_name=username, org_id=org_id, origin_url=origin_url)
159159
response, status = "", HTTPStatus.NO_CONTENT
160160
except BusinessException as exception:
161161
response, status = {"code": exception.code, "message": exception.message}, exception.status_code

auth-api/src/auth_api/services/user.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,9 @@ def _create_new_user_and_membership(db_username, kc_user, membership, org_id):
238238

239239
@staticmethod
240240
@user_context
241-
def delete_otp_for_user(user_name, origin_url: str = None, **kwargs):
241+
def delete_otp_for_user(user_name, org_id, origin_url: str = None, **kwargs):
242242
"""Reset the OTP of the user."""
243-
# TODO - handle when the multiple teams implemented for bceid..
244243
user = UserModel.find_by_username(user_name)
245-
membership = MembershipModel.find_membership_by_userid(user.id)
246-
org_id = membership.org_id
247-
248244
user_from_context: UserContext = kwargs["user_context"]
249245
if not user_from_context.has_role(Role.MANAGE_RESET_OTP.value):
250246
check_auth(org_id=org_id, one_of_roles=(ADMIN, COORDINATOR, STAFF))

auth-api/tests/unit/api/test_cors_preflight.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ def test_preflight_user(app, client, jwt, session):
260260
assert rv.status_code == HTTPStatus.OK
261261
assert_access_control_headers(rv, "*", "POST")
262262

263-
rv = client.options("/api/v1/users/USERNAME/otp", headers={"Access-Control-Request-Method": "DELETE"})
263+
rv = client.options("/api/v1/users/USERNAME/otp/1", headers={"Access-Control-Request-Method": "DELETE"})
264264
assert rv.status_code == HTTPStatus.OK
265265
assert_access_control_headers(rv, "*", "DELETE")
266266

auth-api/tests/unit/api/test_user.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -895,7 +895,7 @@ def test_delete_otp_for_user(client, jwt, session): # pylint:disable=unused-arg
895895

896896
# staff with manage accounts otp reset
897897
headers = factory_auth_header(jwt=jwt, claims=TestJwtClaims.staff_manage_accounts_role)
898-
rv = client.delete(f"api/v1/users/{user.username}/otp", headers=headers)
898+
rv = client.delete(f"api/v1/users/{user.username}/otp/{org.id}", headers=headers)
899899
assert rv.status_code == HTTPStatus.NO_CONTENT
900900

901901
user1 = KEYCLOAK_SERVICE.get_user_by_username(request.user_name)
@@ -904,19 +904,19 @@ def test_delete_otp_for_user(client, jwt, session): # pylint:disable=unused-arg
904904

905905
# staff with basic access cant do otp reset
906906
headers = factory_auth_header(jwt=jwt, claims=TestJwtClaims.staff_role)
907-
rv = client.delete(f"api/v1/users/{user.username}/otp", headers=headers)
907+
rv = client.delete(f"api/v1/users/{user.username}/otp/{org.id}", headers=headers)
908908
assert rv.status_code == HTTPStatus.UNAUTHORIZED
909909

910910
# admin can do otp reset
911-
rv = client.delete(f"api/v1/users/{user.username}/otp", headers=admin_headers)
911+
rv = client.delete(f"api/v1/users/{user.username}/otp/{org.id}", headers=admin_headers)
912912
assert rv.status_code == HTTPStatus.NO_CONTENT
913913

914914
# coordinator can do otp reset
915-
rv = client.delete(f"api/v1/users/{user.username}/otp", headers=coordinator_headers)
915+
rv = client.delete(f"api/v1/users/{user.username}/otp/{org.id}", headers=coordinator_headers)
916916
assert rv.status_code == HTTPStatus.NO_CONTENT
917917

918918
# user can not do otp reset
919-
rv = client.delete(f"api/v1/users/{user.username}/otp", headers=user_headers)
919+
rv = client.delete(f"api/v1/users/{user.username}/otp/{org.id}", headers=user_headers)
920920
assert rv.status_code == HTTPStatus.FORBIDDEN
921921

922922
# another org admin cant do
@@ -925,7 +925,7 @@ def test_delete_otp_for_user(client, jwt, session): # pylint:disable=unused-arg
925925
factory_membership_model(admin_user1.id, org1.id)
926926
admin_claims = TestJwtClaims.get_test_real_user(admin_user1.keycloak_guid)
927927
admin1_headers = factory_auth_header(jwt=jwt, claims=admin_claims)
928-
rv = client.delete(f"api/v1/users/{user.username}/otp", headers=admin1_headers)
928+
rv = client.delete(f"api/v1/users/{user.username}/otp/{org.id}", headers=admin1_headers)
929929
assert rv.status_code == HTTPStatus.FORBIDDEN
930930

931931

auth-api/tests/unit/services/test_user.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ def test_delete_otp_for_user(session, auth_mock, keycloak_mock, monkeypatch):
180180
factory_membership_model(user.id, org.id)
181181

182182
patch_token_info(admin_claims, monkeypatch)
183-
UserService.delete_otp_for_user(user.username)
183+
UserService.delete_otp_for_user(user.username, org.id)
184184
user1 = kc_service.get_user_by_username(request.user_name)
185185
assert "CONFIGURE_TOTP" in json.loads(user1.value()).get("requiredActions")
186186

auth-web/package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

auth-web/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "auth-web",
3-
"version": "2.10.19",
3+
"version": "2.10.20",
44
"appName": "Auth Web",
55
"sbcName": "SBC Common Components",
66
"private": true,

auth-web/src/components/auth/account-settings/team-management/MemberDataTable.vue

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ export default class MemberDataTable extends Vue {
280280
private readonly currentMembership!: Member
281281
private readonly currentOrganization!: Organization
282282
private readonly getRoleInfo!: () => Promise<RoleInfo[]>
283-
private readonly resetOTPAuthenticator!: (username: string) => any
283+
private readonly resetOTPAuthenticator!: (username: string, org_id: number) => any
284284
private readonly roleInfos!: RoleInfo[]
285285
private readonly SNACKBAR_TIMEOUT: number = 3000 // milliseconds
286286
private confirmResetAuthDialog = false
@@ -582,7 +582,7 @@ export default class MemberDataTable extends Vue {
582582
583583
private async resetAuthenticator () {
584584
try {
585-
await this.resetOTPAuthenticator(this.selectedUserForReset?.user?.username)
585+
await this.resetOTPAuthenticator(this.selectedUserForReset?.user?.username, this.currentOrganization?.id)
586586
this.showResetSnackBar = true
587587
this.$refs.resetAuthenticatorDialog.close()
588588
// wait for the SNACKBAR_TIMEOUT value before unsetting the selectedUserForReset,

auth-web/src/composables/transactions-factory.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export const useTransactions = () => {
8181
console.error('Failed to get transaction list.', error)
8282
}
8383
transactions.loading = false
84-
}, 1000, { leading: true, trailing: true }) as (filterField?: string, value?: any, viewAll?: boolean) => Promise<void>
84+
}, 2000, { leading: true, trailing: true }) as (filterField?: string, value?: any, viewAll?: boolean) => Promise<void>
8585

8686
const getTransactionReport = async () => {
8787
try {

auth-web/src/services/user.services.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,8 +113,8 @@ export default class UserService {
113113
return axios.get(`${ConfigHelper.getAuthAPIUrl()}/users/${encodeURIComponent(userGuid)}/affidavits`, { params })
114114
}
115115

116-
static async resetOTPAuthenticator (username: string): Promise<AxiosResponse<any>> {
117-
return axios.delete(`${ConfigHelper.getAuthAPIUrl()}/users/${username}/otp`)
116+
static async resetOTPAuthenticator (username: string, orgId: number): Promise<AxiosResponse<any>> {
117+
return axios.delete(`${ConfigHelper.getAuthAPIUrl()}/users/${username}/otp/${orgId}`)
118118
}
119119

120120
static getUserAccountSettings (currentUserSub: string): Promise<AxiosResponse<UserSettings[]>> {

0 commit comments

Comments
 (0)