Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/11 combine profile update and fi association #38

Merged
merged 15 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/entities/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
"FinanicialInstitutionAssociationDto",
"DeniedDomainDao",
"DeniedDomainDto",
"UserProfile",
"AuthenticatedUser",
]

Expand All @@ -25,5 +26,6 @@
FinancialInsitutionDomainCreate,
FinanicialInstitutionAssociationDto,
DeniedDomainDto,
UserProfile,
AuthenticatedUser,
)
15 changes: 13 additions & 2 deletions src/entities/models/dto.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from typing import Any, Dict, List

from typing import List, Dict, Any, Set, Optional
from pydantic import BaseModel
from starlette.authentication import BaseUser

Expand Down Expand Up @@ -41,6 +40,18 @@ class Config:
orm_mode = True


class UserProfile(BaseModel):
first_name: str
last_name: str
leis: Optional[Set[str]]

def to_keycloak_user(self):
return {"firstName": self.first_name, "lastName": self.last_name}

class Config:
allow_population_by_field_name = True


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be removed now too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

class FinanicialInstitutionAssociationDto(FinancialInstitutionDto):
approved: bool

Expand Down
6 changes: 5 additions & 1 deletion src/oauth2/oauth2_admin.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from http import HTTPStatus
import logging
import os
from typing import Dict, Any
from typing import Dict, Any, Set

import jose.jwt
import requests
Expand Down Expand Up @@ -88,5 +88,9 @@ def associate_to_lei(self, user_id: str, lei: str) -> None:
detail="No institution found for given LEI",
)

def associate_to_leis(self, user_id: str, leis: Set[str]):
for lei in leis:
self.associate_to_lei(user_id, lei)


oauth2_admin = OAuth2Admin()
16 changes: 9 additions & 7 deletions src/routers/admin.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
from http import HTTPStatus
from typing import Dict, Any, Set
from typing import Set
from fastapi import Depends, Request
from starlette.authentication import requires
from dependencies import check_domain
from util import Router
from entities.models import UserProfile

from entities.models import AuthenticatedUser
from oauth2 import oauth2_admin
Expand All @@ -13,18 +14,19 @@

@router.get("/me/", response_model=AuthenticatedUser)
@requires("authenticated")
async def get_me(request: Request):
def get_me(request: Request):
return request.user


@router.put("/me/", status_code=HTTPStatus.ACCEPTED, dependencies=[Depends(check_domain)])
@requires("manage-account")
async def update_me(request: Request, user: Dict[str, Any]):
oauth2_admin.update_user(request.user.id, user)
def update_me(request: Request, user: UserProfile):
oauth2_admin.update_user(request.user.id, user.to_keycloak_user())
if user.leis:
oauth2_admin.associate_to_leis(request.user.id, user.leis)


@router.put("/me/institutions/", status_code=HTTPStatus.ACCEPTED, dependencies=[Depends(check_domain)])
@requires("manage-account")
async def associate_lei(request: Request, leis: Set[str]):
for lei in leis:
oauth2_admin.associate_to_lei(request.user.id, lei)
def associate_lei(request: Request, leis: Set[str]):
oauth2_admin.associate_to_leis(request.user.id, leis)
34 changes: 21 additions & 13 deletions tests/api/routers/test_admin_api.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from unittest.mock import Mock, call
from unittest.mock import Mock

from fastapi import FastAPI
from fastapi.testclient import TestClient
Expand Down Expand Up @@ -40,7 +40,7 @@ def test_get_me_authed_with_institutions(self, app_fixture: FastAPI, auth_mock:

def test_update_me_unauthed(self, app_fixture: FastAPI, unauthed_user_mock: Mock):
client = TestClient(app_fixture)
res = client.put("/v1/admin/me", json={"firstName": "testFirst", "lastName": "testLast"})
res = client.put("/v1/admin/me", json={"first_name": "testFirst", "last_name": "testLast", "leis": ["testLei"]})
assert res.status_code == 403

def test_update_me_no_permission(self, app_fixture: FastAPI, auth_mock: Mock):
Expand All @@ -55,27 +55,35 @@ def test_update_me_no_permission(self, app_fixture: FastAPI, auth_mock: Mock):
AuthenticatedUser.from_claim(claims),
)
client = TestClient(app_fixture)
res = client.put("/v1/admin/me", json={"firstName": "testFirst", "lastName": "testLast"})
res = client.put("/v1/admin/me", json={"first_name": "testFirst", "last_name": "testLast", "leis": ["testLei"]})
assert res.status_code == 403

def test_update_me(self, mocker: MockerFixture, app_fixture: FastAPI, authed_user_mock: Mock):
update_user_mock = mocker.patch("oauth2.oauth2_admin.OAuth2Admin.update_user")
associate_lei_mock = mocker.patch("oauth2.oauth2_admin.OAuth2Admin.associate_to_leis")
update_user_mock.return_value = None
associate_lei_mock.return_value = None
client = TestClient(app_fixture)
data = {"firstName": "testFirst", "lastName": "testLast"}
data = {"first_name": "testFirst", "last_name": "testLast", "leis": ["testLei1", "testLei2"]}
res = client.put("/v1/admin/me", json=data)
update_user_mock.assert_called_once_with("testuser123", data)
update_user_mock.assert_called_once_with("testuser123", {"firstName": "testFirst", "lastName": "testLast"})
associate_lei_mock.assert_called_once_with("testuser123", {"testLei1", "testLei2"})
assert res.status_code == 202

def test_update_me_no_lei(self, mocker: MockerFixture, app_fixture: FastAPI, authed_user_mock: Mock):
update_user_mock = mocker.patch("oauth2.oauth2_admin.OAuth2Admin.update_user")
associate_lei_mock = mocker.patch("oauth2.oauth2_admin.OAuth2Admin.associate_to_leis")
update_user_mock.return_value = None
client = TestClient(app_fixture)
res = client.put("/v1/admin/me", json={"first_name": "testFirst", "last_name": "testLast"})
update_user_mock.assert_called_once_with("testuser123", {"firstName": "testFirst", "lastName": "testLast"})
lchen-2101 marked this conversation as resolved.
Show resolved Hide resolved
associate_lei_mock.assert_not_called()
assert res.status_code == 202

def test_associate_institutions(self, mocker: MockerFixture, app_fixture: FastAPI, authed_user_mock: Mock):
associate_lei_mock = mocker.patch("oauth2.oauth2_admin.OAuth2Admin.associate_to_lei")
associate_lei_mock = mocker.patch("oauth2.oauth2_admin.OAuth2Admin.associate_to_leis")
associate_lei_mock.return_value = None
client = TestClient(app_fixture)
data = ["testlei1", "testlei2"]
res = client.put("/v1/admin/me/institutions", json=data)
expected_calls = [
call("testuser123", "testlei1"),
call("testuser123", "testlei2"),
]
associate_lei_mock.assert_has_calls(expected_calls, any_order=True)
res = client.put("/v1/admin/me/institutions", json=["testlei1", "testlei2"])
associate_lei_mock.assert_called_once_with("testuser123", {"testlei1", "testlei2"})
assert res.status_code == 202