comments for targeting rfc 9635

README.md

@@ -1,4 +1,13 @@
 # sunet-auth-server
-oauth.xyz/GNAP auth server
+### GNAP auth server
 
-This implementation targets draft-ietf-gnap-core-protocol-10.
+This implementation targets RFC 9635.
+
+### Notable features missing:
+* HTTPSIG support
+* Access token introspection
+* Access token revocation
+* Access token rotation
+* Grant revocation
+* Grant modification
+* Correct error responses

src/auth_server/flows.py

@@ -158,9 +158,9 @@ async def check_proof(self, gnap_key: Key, gnap_request: Optional[Union[GrantReq
             if not self.request.context.client_cert:
                 raise NextFlowException(status_code=400, detail="no client certificate found")
             return await check_mtls_proof(gnap_key=gnap_key, cert=self.request.context.client_cert)
-        # HTTPSIGN
-        elif gnap_key.proof.method is ProofMethod.HTTPSIGN:
-            raise NextFlowException(status_code=400, detail="httpsign is not implemented")
+        # HTTPSIG
+        elif gnap_key.proof.method is ProofMethod.HTTPSIG:
+            raise NextFlowException(status_code=400, detail="httpsig proof method is not implemented")
         # JWS
         elif gnap_request and gnap_key.proof.method is ProofMethod.JWS:
             return await check_jws_proof(

src/auth_server/models/gnap.py

@@ -28,12 +28,10 @@ class GnapBaseModel(BaseModel):
 
 
 class ProofMethod(str, Enum):
-    DPOP = "dpop"
-    HTTPSIGN = "httpsign"
+    HTTPSIG = "httpsig"
+    MTLS = "mtls"
     JWSD = "jwsd"
     JWS = "jws"
-    MTLS = "mtls"
-    OAUTHPOP = "oauthpop"
     TEST = "test"
 
 
@@ -155,6 +153,29 @@ class User(GnapBaseModel):
     assertions: Optional[List[SubjectAssertion]] = None
 
 
+class TokenManagementInfo(GnapBaseModel):
+    # TODO:
+    #    uri (string):  The URI of the token management API for this access
+    #       token.  This URI MUST be an absolute URI.  This URI MUST NOT
+    #       include the value of the access token being managed or the value
+    #       of the access token used to protect the URI.  This URI SHOULD be
+    #       different for each access token issued in a request.  REQUIRED.
+    #    access_token (object):  A unique access token for continuing the
+    #       request, called the "token management access token".  The value of
+    #       this property MUST be an object in the format specified in
+    #       Section 3.2.1.  This access token MUST be bound to the client
+    #       instance's key used in the request (or its most recent rotation)
+    #       and MUST NOT be a bearer token.  As a consequence, the flags array
+    #       of this access token MUST NOT contain the string bearer, and the
+    #       key field MUST be omitted.  This access token MUST NOT have a
+    #       manage field.  This access token MUST NOT have the same value as
+    #       the token it is managing.  The client instance MUST present the
+    #       continuation access token in all requests to the continuation URI
+    #       as described in Section 7.2.  REQUIRED.
+    uri: Optional[str] = None
+    access_token: Optional[Any] = None
+
+
 class StartInteractionMethod(str, Enum):
     REDIRECT = "redirect"
     APP = "app"
@@ -229,7 +250,7 @@ class InteractionResponse(GnapBaseModel):
 class AccessTokenResponse(GnapBaseModel):
     value: str
     label: Optional[str] = None
-    manage: Optional[str] = None
+    manage: Optional[TokenManagementInfo] = None
     access: Optional[List[Union[str, Access]]] = None
     expires_in: Optional[int] = Field(default=None, description="seconds until expiry")
     key: Optional[Union[str, Key]] = None
@@ -243,19 +264,26 @@ class SubjectResponse(GnapBaseModel):
 
 
 class ErrorCode(str, Enum):
+    INVALID_REQUEST = "invalid_request"
     INVALID_CLIENT = "invalid_client"
     INVALID_INTERACTION = "invalid_interaction"
-    INVALID_REQUEST = "invalid_request"
+    INVALID_FLAG = "invalid_flag"
+    INVALID_ROTATION = "invalid_rotation"
+    KEY_ROTATION_NOT_SUPPORTED = "key_rotation_not_supported"
+    INVALID_CONTINUATION = "invalid_continuation"
+    USER_DENIED = "user_denied"
     REQUEST_DENIED = "request_denied"
+    UNKNOWN_USER = "unknown_user"
+    UNKNOWN_INTERACTION = "unknown_interaction"
     TOO_FAST = "too_fast"
-    UNKNOWN_REQUEST = "unknown_request"
-    USER_DENIED = "user_denied"
+    TOO_MANY_ATTEMPTS = "too_many_attempts"
 
 
 # TODO: Change FastApi HTTPException responses to ErrorResponse
 class ErrorResponse(BaseModel):
-    error: ErrorCode
+    code: ErrorCode
     error_description: Optional[str] = None
+    continue_: Optional[Continue] = Field(default=None, alias="continue")
 
 
 class ContinueRequest(GnapBaseModel):

src/auth_server/routers/interaction.py

@@ -64,6 +64,14 @@ async def user_code_finish(request: ContextRequest, user_code: Optional[str] = F
         # TODO: show error in template
         return templates.TemplateResponse("user_code.jinja2", context={"request": request})
 
+    # normalize user code
+    # the AS MUST transform the input string remove invalid characters
+    # the AS MUST treat user input as case-insensitive
+    user_code = "".join(user_code.split()).lower()
+    if not user_code.isalnum():
+        # TODO: show error in template
+        return templates.TemplateResponse("user_code.jinja2", context={"request": request})
+
     transaction_state = await transaction_db.get_state_by_user_code(user_code)
     if transaction_state is None:
         raise HTTPException(status_code=404, detail="transaction not found")
@@ -102,6 +110,11 @@ async def finish_interaction(
             ),
         )
 
+        # TODO: The client instance's URI MUST be protected by HTTPS, be hosted on a
+        #       server local to the RO's browser ("localhost"), or use an
+        #       application-specific URI scheme that is loaded on the end user's
+        #       device.
+
         # redirect method
         if transaction_state.grant_request.interact.finish.method is FinishInteractionMethod.REDIRECT:
             redirect_url = f"{transaction_state.grant_request.interact.finish.uri}?hash={interaction_hash}&interact_ref={interact_ref}"

src/auth_server/routers/root.py

@@ -42,6 +42,7 @@ async def get_public_pem(signing_key: JWK = Depends(get_signing_key)):
     return Response(content=data, media_type="application/x-pem-file")
 
 
+# TODO implement OPTIONS (discovery)
 @root_router.post("/transaction", response_model=GrantResponse, response_model_exclude_none=True)
 async def transaction(
     request: ContextRequest,
@@ -84,11 +85,14 @@ async def transaction(
         if isinstance(res, GrantResponse):
             logger.info(f"flow {auth_flow_name} returned GrantResponse")
             logger.debug(res.dict(exclude_none=True))
+            # TODO: The AS MUST include the HTTP Cache-Control response header field
+            #       [RFC9111] with a value set to "no-store".
             return res
 
     raise HTTPException(status_code=401, detail="permission denied")
 
 
+# TODO: implement DELETE (revoke transaction) and PATCH (modify transaction) for continue
 @root_router.post("/continue/{continue_reference}", response_model=GrantResponse, response_model_exclude_none=True)
 @root_router.post("/continue", response_model=GrantResponse, response_model_exclude_none=True)
 async def continue_transaction(
@@ -137,6 +141,11 @@ async def continue_transaction(
     if authorization != f"GNAP {transaction_state.continue_access_token}":
         raise HTTPException(status_code=401, detail="permission denied")
 
+    # TODO: Need to verify that continuation responses are handled correctly
+    # Do not return transaction reference again
+    # Change continuation access token for next request
+    # More?
+
     # return continue response again if interaction is not completed or interaction reference is not used
     if transaction_state.flow_state != FlowState.APPROVED:
         logger.debug(f"transaction state: {transaction_state.flow_state}. Can not continue yet.")
@@ -167,3 +176,6 @@ async def continue_transaction(
         return res
 
     raise HTTPException(status_code=401, detail="permission denied")
+
+
+# TODO: implement token management end point

src/auth_server/saml2.py

@@ -100,9 +100,12 @@ class SessionInfo(BaseModel):
     authn_info: List[AuthnInfo] = Field(default_factory=list)
     name_id: NameID
     attributes: SAMLAttributes
+    # TODO: raw assertion needed for spec compliant SubjectAssertion
+    # raw_assertion: str
 
     @classmethod
     def from_pysaml2(cls, session_info: Dict[str, Any]) -> SessionInfo:
+        # session_info["raw_assertion"] = raw_assertion
         session_info["authn_info"] = [
             AuthnInfo(authn_class=item[0], authn_authority=item[1], authn_instant=item[2])
             for item in session_info["authn_info"]
@@ -301,6 +304,7 @@ async def process_assertion(saml_response: str) -> Optional[AssertionData]:
         logger.info("Unknown response")
         raise BadSAMLResponse("Unknown response")
 
+    # raw_assertion = str(response.assertions[0])
     session_info = SessionInfo.from_pysaml2(response.session_info())
     assertion_data = AssertionData(session_info=session_info, authn_req_ref=authn_ref)
     # Fix eduPersonTargetedID

src/auth_server/tests/test_app.py

@@ -185,8 +185,12 @@ def _fake_saml_authentication(self, transaction_id: str):
         authn_info = [
             AuthnInfo(authn_class="https://refeds.org/profile/mfa", authn_authority=[], authn_instant=utc_now())
         ]
-        transaction_state.saml_assertion = SessionInfo(
-            issuer="https://idp.example.com", attributes=attributes, name_id=name_id, authn_info=authn_info
+        transaction_state.saml_session_info = SessionInfo(
+            issuer="https://idp.example.com",
+            attributes=attributes,
+            name_id=name_id,
+            authn_info=authn_info,
+            # raw_assertion="mock_raw_assertion",
         )
         self._save_transaction_state(transaction_state)