Skip to content

Commit

Permalink
Merge pull request #376 from bcgov/376-unique-consistent-user-identifier
Browse files Browse the repository at this point in the history
Provide unique, consistent user identifier for repeated authentication attempts when using same credential(s)
  • Loading branch information
esune authored Nov 22, 2023
2 parents ac7c5e7 + 3637c60 commit bc862e2
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 14 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ curl -X 'POST' \
-d '{
"ver_config_id": "verified-email",
"subject_identifier": "email",
"generate_consistent_identifier": true,
"proof_request": {
"name": "BCGov Verified Email",
"version": "1.0",
Expand Down
5 changes: 4 additions & 1 deletion docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ A verifiable credential presentation request configuration, takes the following
{
"id": "<configuration-identifier>",
"subject_identifier": "<attribute-name>",
"generate_consistent_identifier": <true|false>,
"proof_request": {
"name": "Basic Proof",
"version": "1.0",
Expand All @@ -156,6 +157,7 @@ This data model is inspired by that is defined and used in the [Hyperledger Indy

- `id` : The identifier for the presentation configuration.
- `subject_identifier` : See [here](#subject-identifer-mapping) for further details on the purpose of this field.
- `generate_consistent_identifier` : Optional field defaulting to false. See [here](#subject-identifer-mapping) for more details.
- `proof_request` : Contains the details on the presentation request, e.g which attributes are to be disclosed
- `name` : The name that will accompany the presentation request
- `version` : The version of the presentation request
Expand Down Expand Up @@ -308,7 +310,7 @@ To quote from the OpenID Connect specification on [ID tokens](https://openid.net

`sub : REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII characters in length. The sub value is a case sensitive string`

When an OP is performing VC-AuthN, and the request has reached the point where the VC presentation has been generated and sent by the IW to the OP, the OP must now map contents of this VC presentation to an OpenID ID token. The question is then raised on what should populate this field. The two options available are:
When an OP is performing VC-AuthN, and the request has reached the point where the VC presentation has been generated and sent by the IW to the OP, the OP must now map contents of this VC presentation to an OpenID ID token. The question is then raised on what should populate this field. The three options available are:

1. Nominate a disclosed attribute in the verifiable credential presentation that is used to populate the subject field.
2. Ephemeral generate an identifier for this field e.g a randomly generated GUID.
Expand All @@ -317,6 +319,7 @@ When an OP is performing VC-AuthN, and the request has reached the point where t
**Note:**
- In option 2. this prevents the often desirable property of cross session correlation of an authenticated user, which will effect the ability for many integrating IAM solutions being able to conduct effective auditing.
- In option 3. this method should be assessed and used with caution, as the chance of collisions for users holding credentials with exact same values is possible (e.g.: a proof-request using only `first_name` and `last_name`, would generate the same identifier for people with same first and last name).
- In order to enable option 3 the _presentation request configuration_ must have `generate_consistent_identifier`

#### UserInfo Endpoint

Expand Down
6 changes: 5 additions & 1 deletion oidc-controller/api/core/models.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from datetime import datetime
from typing import TypedDict

from bson import ObjectId
from pydantic import BaseModel, Field
Expand Down Expand Up @@ -48,7 +49,10 @@ class GenericErrorMessage(BaseModel):
detail: str


class RevealedAttribute(BaseModel):
# Currently used as a TypedDict since it can be used as a part of a
# Pydantic class but a Pydantic class can not inherit from TypedDict
# and and BaseModel
class RevealedAttribute(TypedDict, total=False):
sub_proof_index: int
values: dict

Expand Down
26 changes: 20 additions & 6 deletions oidc-controller/api/core/oidc/issue_token_service.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import canonicaljson
import dataclasses
import json
from datetime import datetime
Expand All @@ -9,7 +10,7 @@

from ...authSessions.models import AuthSession
from ...verificationConfigs.models import ReqAttr, VerificationConfig
from ..models import RevealedAttribute
from ...core.models import RevealedAttribute

logger = structlog.getLogger(__name__)

Expand Down Expand Up @@ -57,9 +58,10 @@ def get_claims(
referent: str
requested_attr: ReqAttr
try:
for referent, requested_attr in auth_session.presentation_exchange[
for referent, requested_attrdict in auth_session.presentation_exchange[
"presentation_request"
]["requested_attributes"].items():
requested_attr = ReqAttr(**requested_attrdict)
logger.debug(
f"Processing referent: {referent}, requested_attr: {requested_attr}"
)
Expand All @@ -72,7 +74,7 @@ def get_claims(
]
logger.debug(f"revealed_attrs: {revealed_attrs}")
# loop through each value and put it in token as a claim
for attr_name in requested_attr["names"]:
for attr_name in requested_attr.names:
logger.debug(f"AttrName: {attr_name}")
presentation_claims[attr_name] = Claim(
type=attr_name,
Expand All @@ -85,18 +87,30 @@ def get_claims(
)
raise RuntimeError(err)

proof_claims = json.dumps(
{c.type: c.value for c in presentation_claims.values()}
)
# look at all presentation_claims for one
# matching the configured subject_identifier, if any
sub_id_claim = presentation_claims.get(ver_config.subject_identifier)

if sub_id_claim:
# add sub and append presentation_claims
oidc_claims.append(Claim(type="sub", value=sub_id_claim.value))
elif ver_config.generate_consistent_identifier:
# Do not create a sub based on the proof claims if the
# user requests a generated identifier
oidc_claims.append(
Claim(
type="sub",
value=canonicaljson.encode_canonical_json(proof_claims).decode(
"utf-8"
),
)
)

result = {c.type: c.value for c in oidc_claims}
result[PROOF_CLAIMS_ATTRIBUTE_NAME] = json.dumps(
{c.type: c.value for c in presentation_claims.values()}
)
result[PROOF_CLAIMS_ATTRIBUTE_NAME] = proof_claims

# TODO: Remove after full transistion to v2.0
# Add the presentation claims to the result as keys for backwards compatibility [v1.0]
Expand Down
45 changes: 44 additions & 1 deletion oidc-controller/api/core/oidc/tests/test_issue_token_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ async def test_valid_presentation_with_matching_subject_identifier_has_identifie


@pytest.mark.asyncio
async def test_valid_presentation_with_non_matching_subject_identifier_and_has_no_sub():
async def test_valid_presentation_with_non_matching_subject_identifier_and_generate_consistent_identifier_is_missing_and_has_no_sub():
presentation["presentation_request"][
"requested_attributes"
] = basic_valid_requested_attributes
Expand All @@ -229,4 +229,47 @@ async def test_valid_presentation_with_non_matching_subject_identifier_and_has_n
with mock.patch.object(AuthSession, "presentation_exchange", presentation):
ver_config.subject_identifier = "not-email"
claims = Token.get_claims(auth_session, ver_config)
assert not ver_config.generate_consistent_identifier
assert "sub" not in claims


@pytest.mark.asyncio
async def test_valid_presentation_with_non_matching_subject_identifier_and_generate_consistent_identifier_false_and_has_no_sub():
presentation["presentation_request"][
"requested_attributes"
] = basic_valid_requested_attributes
presentation["presentation"]["requested_proof"][
"revealed_attr_groups"
] = basic_valid_revealed_attr_groups
with mock.patch.object(AuthSession, "presentation_exchange", presentation):
ver_config.subject_identifier = "not-email"
ver_config.generate_consistent_identifier = False
claims = Token.get_claims(auth_session, ver_config)
assert "sub" not in claims


@pytest.mark.asyncio
async def test_valid_presentation_with_non_matching_subject_identifier_and_generate_consistent_identifier_true_and_has_sub():
presentation["presentation_request"][
"requested_attributes"
] = basic_valid_requested_attributes
presentation["presentation"]["requested_proof"][
"revealed_attr_groups"
] = basic_valid_revealed_attr_groups
with mock.patch.object(AuthSession, "presentation_exchange", presentation):
ver_config.subject_identifier = "not-email"
ver_config.generate_consistent_identifier = True
claims = Token.get_claims(auth_session, ver_config)
assert "sub" in claims

# Ensure that this sub is not using the ver_config.subject_identifier
ver_config.subject_identifier = "email"
ver_config.generate_consistent_identifier = False
claims_subject_identifier = Token.get_claims(auth_session, ver_config)
assert claims["sub"] != claims_subject_identifier["sub"]

# Ensure that sub is consistent
ver_config.subject_identifier = "not-email"
ver_config.generate_consistent_identifier = True
claims_duplicate = Token.get_claims(auth_session, ver_config)
assert claims["sub"] == claims_duplicate["sub"]
4 changes: 3 additions & 1 deletion oidc-controller/api/routers/oidc.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import base64
import io
from typing import cast
import uuid
from datetime import datetime
from urllib.parse import urlencode
Expand Down Expand Up @@ -163,7 +164,8 @@ async def get_authorize_callback(pid: str, db: Database = Depends(get_db)):
async def post_token(request: Request, db: Database = Depends(get_db)):
"""Called by oidc platform to retrieve token contents"""
async with request.form() as form:
form_dict = form._dict
logger.warn(f"post_token: form was {form}")
form_dict = cast(dict[str, str], form._dict)
auth_session = await AuthSessionCRUD(db).get_by_pyop_auth_code(
form_dict["code"]
)
Expand Down
1 change: 1 addition & 0 deletions oidc-controller/api/verificationConfigs/examples.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
ex_ver_config = {
"ver_config_id": "test-request-config",
"include_v1_attributes": False,
"generate_consistent_identifier": False,
"subject_identifier": "first_name",
"proof_request": {
"name": "Basic Proof",
Expand Down
7 changes: 3 additions & 4 deletions oidc-controller/api/verificationConfigs/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class VerificationProofRequest(BaseModel):
class VerificationConfigBase(BaseModel):
subject_identifier: str = Field()
proof_request: VerificationProofRequest = Field()
generate_consistent_identifier: Optional[bool] = Field(default=False)
include_v1_attributes: Optional[bool] = Field(default=False)

def generate_proof_request(self):
Expand All @@ -52,8 +53,7 @@ def generate_proof_request(self):
}
for i, req_attr in enumerate(self.proof_request.requested_attributes):
label = req_attr.label or "req_attr_" + str(i)
result["requested_attributes"][label] = req_attr.dict(
exclude_none=True)
result["requested_attributes"][label] = req_attr.dict(exclude_none=True)
if settings.SET_NON_REVOKED:
result["requested_attributes"][label]["non_revoked"] = {
"from": int(time.time()),
Expand All @@ -62,8 +62,7 @@ def generate_proof_request(self):
# TODO add I indexing
for req_pred in self.proof_request.requested_predicates:
label = req_pred.label or "req_pred_" + str(i)
result["requested_predicates"][label] = req_pred.dict(
exclude_none=True)
result["requested_predicates"][label] = req_pred.dict(exclude_none=True)
if settings.SET_NON_REVOKED:
result["requested_attributes"][label]["non_revoked"] = {
"from": int(time.time()),
Expand Down
1 change: 1 addition & 0 deletions oidc-controller/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ qrcode[pil]==7.4.2
structlog==23.1.0
uvicorn[standard]==0.22.0
python-socketio==5.8.0 # required to run websockets
canonicaljson==2.0.0 # used to provide unique consistent user identifiers

0 comments on commit bc862e2

Please sign in to comment.