Skip to content

Commit 4b28aef

Browse files
Mpt 6538 fix missing bearer token check proceeding with the creation of a cloud account (#30)
* [WIP] Add initial logic for managing DataSources. Move the service's APIs and files to the API folder. Add Custom Exceptions to handle DataSources errors * Add initial check for validating the Bearer Token passed to the POST `modifier/v1/organizations/{org_id/cloud_accounts`. This prevents the service from returning a 403 with an indication about a not-allowed cloud type instead of processing the wrong authorization as the first thing. Change the error returned in the case of a not-allowed cloud type to 400 * Rename valide_authorization to check_user_allowed_to_create_cloud_account Remove the returned response. Adjust tests
1 parent 16c1ff5 commit 4b28aef

File tree

6 files changed

+140
-8
lines changed

6 files changed

+140
-8
lines changed

app/api/cloud_account/cloud_accounts_manager.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ def select_strategy(self):
5858
error_code="OE0436",
5959
reason=f"{self.type} is not supported",
6060
params=[f"{self.type}"],
61-
status_code=http_status.HTTP_403_FORBIDDEN,
61+
status_code=http_status.HTTP_400_BAD_REQUEST,
6262
)
6363

6464
strategy_class = self.ALLOWED_PROVIDERS[self.type]

app/api/organizations/api.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,16 @@ async def link_cloud_account(
110110
org_id: str,
111111
data: AddCloudAccount,
112112
user_access_token: Annotated[str, Depends(get_bearer_token)],
113+
auth_client: Annotated[OptScaleAuth, Depends(get_auth_client)],
113114
):
114115
try:
116+
# here, we need to validate the bearer token to ensure that any authorization
117+
# related errors will be checked first. In case of an invalid/expired token,
118+
# an APIResponseError with a http statu 401 will re raised
119+
120+
await auth_client.check_user_allowed_to_create_cloud_account(
121+
bearer_token=user_access_token, org_id=org_id
122+
)
115123
response = await link_cloud_account_to_org(
116124
name=data.name,
117125
type=data.type,

app/optscale_api/auth_api.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from app.core.exceptions import UserAccessTokenError, raise_api_response_exception
88

99
AUTH_TOKEN_ENDPOINT = "/auth/v2/tokens" # nosec B105
10+
AUTH_TOKEN_AUTHORIZE_ENDPOINT = "/auth/v2/authorize" # nosec B105"
1011
logger = logging.getLogger(__name__)
1112

1213

@@ -34,6 +35,37 @@ class OptScaleAuth:
3435
def __init__(self):
3536
self.api_client = APIClient(base_url=settings.opt_scale_api_url)
3637

38+
async def check_user_allowed_to_create_cloud_account(
39+
self, bearer_token: str, org_id: str
40+
) -> None | Exception:
41+
"""
42+
It validates the given bearer_token by making a request
43+
to auth/v2/authorize.
44+
This is used to protect the /organization/{org_id}/cloud_accounts
45+
endpoint from being consumed with a not valid bearer token and fix
46+
the following corner case scenario:
47+
- Wrong Authentication Bearer Token or an expired one.
48+
- Wrong value in the type field of the payload.
49+
Result: The service will return a 403 with an indication about a
50+
not-allowed cloud type instead of processing the wrong authorization
51+
as the first thing.
52+
:param bearer_token: The user access's token
53+
:param org_id: The org ID that owned by the user identified by the bearer_token
54+
:return:
55+
"""
56+
payload = {
57+
"action": "MANAGE_CLOUD_CREDENTIALS",
58+
"resource_type": "organization",
59+
"uuid": org_id,
60+
}
61+
headers = build_bearer_token_header(bearer_token=bearer_token)
62+
response = await self.api_client.post(
63+
endpoint=AUTH_TOKEN_AUTHORIZE_ENDPOINT, headers=headers, data=payload
64+
)
65+
if response.get("error"):
66+
logger.error("Failed validate the given bearer token")
67+
return raise_api_response_exception(response)
68+
3769
async def obtain_user_auth_token_with_admin_api_key(
3870
self, user_id: str, admin_api_key: str
3971
) -> str | Exception:

tests/data/data.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,22 @@
123123
"error": "HTTP error: 401 - {\"error\": {\"status_code\": 401, \"error_code\": \"OE0235\", \"reason\": \"Unauthorized\", \"params\": []}}",
124124
"status_code": 401
125125
}
126+
},
127+
"authorize": {
128+
"error_response": {"data":
129+
{"error": {
130+
"error_code": "OA0010", "params": [],
131+
"reason": "Token not found", "status_code": 401}},
132+
"error": "HTTP error: 401 - {\"error\": {\"status_code\": 401, \"error_code\": \"OA0010\", \"reason\": \"Token not found\", \"params\": []}}",
133+
"status_code": 401},
134+
"valid_response": {"data":
135+
{"assignments": [
136+
{"id": "user_id",
137+
"resource": "org_id",
138+
"role": "Manager",
139+
"type": "organization",
140+
"user": "peter.parker@spiderman.com"}],
141+
"status": "ok"}, "status_code": 200}
126142
}
127143
},
128144
"invitation": {

tests/test_modifier_cloud_accounts_api.py

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,22 @@
88
CloudStrategyManager,
99
)
1010
from app.core.exceptions import APIResponseError
11+
from app.optscale_api.auth_api import OptScaleAuth
12+
13+
14+
@pytest.fixture
15+
def opt_scale_auth():
16+
return OptScaleAuth()
17+
18+
19+
@pytest.fixture
20+
def mock_auth_post():
21+
patcher = patch.object(
22+
OptScaleAuth, "check_user_allowed_to_create_cloud_account", new=AsyncMock()
23+
)
24+
mock = patcher.start()
25+
yield mock
26+
patcher.stop()
1127

1228

1329
@pytest.fixture
@@ -19,10 +35,10 @@ def mock_add_cloud_account():
1935

2036

2137
async def test_link_cloud_account(
22-
async_client: AsyncClient,
23-
test_data: dict,
24-
mock_add_cloud_account,
38+
async_client: AsyncClient, test_data: dict, mock_add_cloud_account, mock_auth_post
2539
):
40+
mock_response = test_data["auth_token"]["authorize"]["valid_response"]
41+
mock_auth_post.return_value = mock_response
2642
payload = test_data["cloud_accounts_conf"]["create"]["data"]["azure"]["conf"]
2743
mocked_response = {
2844
"data": test_data["cloud_accounts_conf"]["create"]["data"]["azure"]["response"]
@@ -44,10 +60,10 @@ async def test_link_cloud_account(
4460

4561

4662
async def test_create_datasource_with_inject_conf(
47-
async_client: AsyncClient,
48-
test_data: dict,
49-
mock_add_cloud_account,
63+
async_client: AsyncClient, test_data: dict, mock_add_cloud_account, mock_auth_post
5064
):
65+
mock_response = test_data["auth_token"]["authorize"]["valid_response"]
66+
mock_auth_post.return_value = mock_response
5167
payload = test_data["cloud_accounts_conf"]["create"]["data"]["azure"]["conf"]
5268
mocked_response = {
5369
"data": test_data["cloud_accounts_conf"]["create"]["data"]["azure"]["response"]
@@ -73,15 +89,18 @@ async def test_not_allowed_datasource_exception_handling(
7389
test_data: dict,
7490
caplog,
7591
mock_add_cloud_account,
92+
mock_auth_post,
7693
):
94+
mock_response = test_data["auth_token"]["authorize"]["valid_response"]
95+
mock_auth_post.return_value = mock_response
7796
payload = test_data["cloud_accounts_conf"]["create"]["data"]["azure"]["conf"]
7897
payload["type"] = "blalbla"
7998
response = await async_client.post(
8099
"/organizations/my_org_id/cloud_accounts",
81100
json=payload,
82101
headers={"Authorization": "Bearer good token"},
83102
)
84-
assert response.status_code == 403
103+
assert response.status_code == 400
85104
got = response.json()
86105
assert got.get("error").get("reason") == "blalbla is not supported"
87106
assert got.get("error").get("error_code") == "OE0436"
@@ -92,7 +111,10 @@ async def test_exception_handling(
92111
test_data: dict,
93112
caplog,
94113
mock_add_cloud_account,
114+
mock_auth_post,
95115
):
116+
mock_response = test_data["auth_token"]["authorize"]["valid_response"]
117+
mock_auth_post.return_value = mock_response
96118
mock_add_cloud_account.side_effect = APIResponseError(
97119
title="Error response from OptScale",
98120
reason="Test Exception",
@@ -121,7 +143,10 @@ async def test_no_auth(
121143
test_data: dict,
122144
caplog,
123145
mock_add_cloud_account,
146+
mock_auth_post,
124147
):
148+
mock_response = test_data["auth_token"]["authorize"]["error_response"]
149+
mock_auth_post.return_value = mock_response
125150
mock_add_cloud_account.side_effect = APIResponseError(
126151
title="Error response from OptScale",
127152
reason="Test Exception",

tests/test_optscle_auth_api.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,57 @@ def mock_get(mocker, opt_scale_auth):
2525
return mock_get
2626

2727

28+
async def test_authorize_valid_bearer_token(
29+
async_client: AsyncClient, test_data: dict, mock_post, opt_scale_auth
30+
):
31+
mock_response = test_data["auth_token"]["authorize"]["valid_response"]
32+
mock_post.return_value = mock_response
33+
response = await opt_scale_auth.check_user_allowed_to_create_cloud_account(
34+
bearer_token="good token", org_id="my_org_id"
35+
)
36+
assert response is None
37+
mock_post.assert_called_once_with(
38+
endpoint="/auth/v2/authorize",
39+
headers={
40+
"Authorization": "Bearer good token",
41+
},
42+
data={
43+
"action": "MANAGE_CLOUD_CREDENTIALS",
44+
"resource_type": "organization",
45+
"uuid": "my_org_id",
46+
},
47+
)
48+
49+
50+
async def test_authorize_bearer_token_with_error(
51+
async_client: AsyncClient, mock_post, test_data: dict, opt_scale_auth, caplog
52+
):
53+
mock_response = test_data["auth_token"]["authorize"]["error_response"]
54+
mock_post.return_value = mock_response
55+
with caplog.at_level(logging.ERROR):
56+
with pytest.raises(APIResponseError) as exc_info:
57+
await opt_scale_auth.check_user_allowed_to_create_cloud_account(
58+
bearer_token="no good token", org_id="my_org_id"
59+
)
60+
exception = exc_info.value
61+
assert exception.error.get("error_code") == "OA0010"
62+
assert exception.error.get("reason") == "Token not found"
63+
assert exception.error.get("status_code") == 401
64+
# check the logging message printed by the obtain_user_auth_token_with_admin_api_key
65+
assert "Failed validate the given bearer token" == caplog.messages[0]
66+
mock_post.assert_called_once_with(
67+
endpoint="/auth/v2/authorize",
68+
headers={
69+
"Authorization": "Bearer no good token",
70+
},
71+
data={
72+
"action": "MANAGE_CLOUD_CREDENTIALS",
73+
"resource_type": "organization",
74+
"uuid": "my_org_id",
75+
},
76+
)
77+
78+
2879
async def test_user_auth_token_with_admin_api_key(
2980
async_client: AsyncClient, test_data: dict, mock_post, opt_scale_auth
3081
):

0 commit comments

Comments
 (0)