Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions src/sentry/core/endpoints/scim/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ class SCIMErrorResponse(TypedDict, total=False):

SCIM_404_USER_RES = {
"schemas": [SCIM_API_ERROR],
"status": "404",
"detail": "User not found.",
}

SCIM_404_GROUP_RES = {
"schemas": [SCIM_API_ERROR],
"status": "404",
"detail": "Group not found.",
}

Expand All @@ -37,26 +39,31 @@ class SCIMErrorResponse(TypedDict, total=False):

SCIM_400_INVALID_FILTER = {
"schemas": [SCIM_API_ERROR],
"status": "400",
"scimType": "invalidFilter",
}

SCIM_400_INTEGRITY_ERROR = {
"schemas": [SCIM_API_ERROR],
"status": "400",
"detail": "Database Integrity Error.",
}

SCIM_400_TOO_MANY_PATCH_OPS_ERROR = {
"schemas": [SCIM_API_ERROR],
"status": "400",
"detail": "Too many patch ops sent, limit is 100.",
}

SCIM_400_UNSUPPORTED_ATTRIBUTE = {
"schemas": [SCIM_API_ERROR],
"status": "400",
"detail": "Invalid Replace attr. Only displayName and members supported.",
}

SCIM_400_INVALID_PAYLOAD = {
"schemas": [SCIM_API_ERROR],
"status": "400",
"detail": "Invalid SCIM payload.",
}

Expand Down
8 changes: 6 additions & 2 deletions src/sentry/core/endpoints/scim/members.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,8 +618,12 @@ def post(
("is already a member" in error) for error in serializer.errors["email"]
):
# we include conflict logic in the serializer, check to see if that was
# our error and if so, return a 409 so the scim IDP knows how to handle
raise SCIMApiError(detail=SCIM_409_USER_EXISTS, status_code=409)
# our error and if so, return a 409 so the scim IDP knows how to handle.
# "uniqueness" tells a spec-compliant IdP this is a duplicate so it can
# fall back from POST to PATCH/PUT against the existing member.
raise SCIMApiError(
detail=SCIM_409_USER_EXISTS, status_code=409, scim_type="uniqueness"
)
if "role" in serializer.errors:
# TODO: Change this to an error pointing to a doc showing the workaround if they
# tried to provision an org admin
Expand Down
10 changes: 4 additions & 6 deletions src/sentry/core/endpoints/scim/teams.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,12 +295,10 @@ def post(
sender=None,
)
except (IntegrityError, MaxSnowflakeRetryError):
return Response(
{
"non_field_errors": [CONFLICTING_SLUG_ERROR],
"detail": CONFLICTING_SLUG_ERROR,
},
status=409,
# A duplicate slug is a uniqueness collision; surface it as a
# spec-compliant SCIM error so IdPs can reconcile the existing team.
raise SCIMApiError(
detail=CONFLICTING_SLUG_ERROR, status_code=409, scim_type="uniqueness"
)

self.create_audit_entry(
Expand Down
11 changes: 9 additions & 2 deletions src/sentry/core/endpoints/scim/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,18 @@


class SCIMApiError(APIException):
def __init__(self, detail, status_code=400):
def __init__(self, detail, status_code=400, scim_type=None):
transaction = sentry_sdk.get_current_scope().transaction
if transaction is not None:
transaction.set_tag("http.status_code", status_code)
super().__init__({"schemas": [SCIM_API_ERROR], "detail": detail})
error_body = {
"schemas": [SCIM_API_ERROR],
"status": str(status_code),
"detail": detail,
}
if scim_type is not None:
error_body["scimType"] = scim_type
super().__init__(error_body)
self.status_code = status_code


Expand Down
8 changes: 8 additions & 0 deletions tests/sentry/core/endpoints/scim/test_scim_team_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def test_team_details_404(self) -> None:

assert response.data == {
"detail": "Group not found.",
"status": "404",
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
}

Expand Down Expand Up @@ -420,6 +421,7 @@ def test_team_member_doesnt_exist_add_to_team(self) -> None:
)
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "404",
"detail": "User not found.",
}

Expand All @@ -432,6 +434,7 @@ def test_team_details_patch_too_many_ops(self) -> None:
assert response.status_code == 400, response.data
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Too many patch ops sent, limit is 100.",
}

Expand All @@ -448,6 +451,7 @@ def test_team_details_invalid_filter_patch_route(self) -> None:

assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"scimType": "invalidFilter",
}

Expand All @@ -464,6 +468,7 @@ def test_add_members_invalid_value_format(self) -> None:
)
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Invalid SCIM payload.",
}

Expand All @@ -480,6 +485,7 @@ def test_add_members_non_numeric_value(self) -> None:
)
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Invalid SCIM payload.",
}

Expand All @@ -496,6 +502,7 @@ def test_replace_members_invalid_value_format(self) -> None:
)
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Invalid SCIM payload.",
}

Expand All @@ -512,6 +519,7 @@ def test_replace_members_non_numeric_value(self) -> None:
)
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Invalid SCIM payload.",
}

Expand Down
8 changes: 7 additions & 1 deletion tests/sentry/core/endpoints/scim/test_scim_team_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ def test_scim_invalid_filter(self) -> None:
assert response.status_code == 400, response.data
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"scimType": "invalidFilter",
}

Expand Down Expand Up @@ -230,7 +231,12 @@ def test_scim_team_no_duplicate_names(self) -> None:
response = self.get_error_response(
self.organization.slug, **self.post_data, status_code=409
)
assert response.data["detail"] == "A team with this slug already exists."
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"scimType": "uniqueness",
"status": "409",
"detail": "A team with this slug already exists.",
}

def test_scim_team_invalid_numeric_slug(self) -> None:
invalid_post_data = {**self.post_data, "displayName": "1234"}
Expand Down
3 changes: 3 additions & 0 deletions tests/sentry/core/endpoints/scim/test_scim_user_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,7 @@ def test_patch_bad_schema(self) -> None:
)
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Invalid Patch Operation.",
}

Expand All @@ -604,6 +605,7 @@ def test_patch_bad_schema(self) -> None:
)
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Invalid Patch Operation.",
}

Expand All @@ -620,6 +622,7 @@ def test_member_detail_patch_too_many_ops(self) -> None:
assert response.status_code == 400, response.data
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": '{"Operations":["Ensure this field has no more than 100 elements."]}',
}

Expand Down
19 changes: 19 additions & 0 deletions tests/sentry/core/endpoints/scim/test_scim_user_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,28 @@ def test_post_users_already_exists(self) -> None:
assert response.status_code == 409, response.content
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"scimType": "uniqueness",
"status": "409",
"detail": "User already exists in the database.",
}
assert not member.flags["idp:provisioned"]
assert not member.flags["idp:role-restricted"]

def test_scim_error_includes_status_without_scim_type(self) -> None:
# Non-uniqueness SCIM errors must still carry a string `status` per
# RFC 7644 §3.12, but must not claim a `scimType` they don't have.
data = post_data()
data["sentryOrgRole"] = "nonexistant"
response = self.get_error_response(
self.organization.slug, method="post", status_code=400, **data
)
assert response.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Invalid organization role.",
}
assert "scimType" not in response.data

def test_post_users_with_role_valid(self) -> None:
data = post_data()
data["sentryOrgRole"] = "manager"
Expand Down Expand Up @@ -268,6 +285,7 @@ def test_post_users_with_role_invalid(self) -> None:
)
assert resp.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Invalid organization role.",
}

Expand All @@ -278,6 +296,7 @@ def test_post_users_with_role_invalid(self) -> None:
)
assert resp.data == {
"schemas": ["urn:ietf:params:scim:api:messages:2.0:Error"],
"status": "400",
"detail": "Invalid organization role.",
}

Expand Down
Loading