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

Maintenance Iteration 21 Holistic Feedback #663

Open
CDR-API-Stream opened this issue Sep 11, 2024 · 17 comments
Open

Maintenance Iteration 21 Holistic Feedback #663

CDR-API-Stream opened this issue Sep 11, 2024 · 17 comments
Labels
Non-breaking change A change that is not expected to result in a new endpoint version.
Milestone

Comments

@CDR-API-Stream
Copy link
Collaborator

This change request has been created to simplify the raising of minor changes, such as text corrections or description clarifications, that are not really material to the standards but do have a real impact on readability and clarity.

Please raise any such suggestions that you would like included in Maintenance Iteration 21 on this issue and the DSB will review them. If a suggestion is a material change a dedicated change request will be raised.

@nils-work
Copy link
Member

The Energy specs don't have servers[...].url detail specified currently. This is inconsistent with other specs and means the 'Code samples' examples don't display a 'Host' property and a full URL.

The Energy Data Holder spec could be updated with https://data.holder.com.au/cds-au/v1 like other specs, or all specs could be updated to something such as https://dh.example.com/cds-au/v1

The Energy Secondary Data Holder spec could be updated with the actual hosts (including AEMO and MarketNet pre-prod and prod options) or a generic default such as https://sdh.example.com/cds-au/v1.

References to recipient.com.au could also be updated to adr.example.com where applicable.

Differences between TLS and MTLS endpoints could also be indicated, for example:

  • https://tls.dh.example.com/cds-au/v1
  • https://mtls.dh.example.com/cds-au/v1

@nils-work nils-work added the Non-breaking change A change that is not expected to result in a new endpoint version. label Sep 18, 2024
@perlboy
Copy link

perlboy commented Sep 19, 2024

In https://consumerdatastandardsaustralia.github.io/standards/#cdr-dynamic-client-registration-api_schemas_tocSclientregistration it states:

Predefined error code as described in section 3.3 OIDC Dynamic Client Registration

But the overriding specification for DCR is RFC7591 and the CTS is now enforcing these error codes explicitly, which is noteworthy because 3.3 OIDC Dynamic Client Registration explicitly states "Other error codes MAY also be used."

@nils-work
Copy link
Member

The Reporting Requirements section contains details that are out of sync with the latest Get Metrics v5 requirements.

The text could be updated from:

The mechanism for reporting will be via the CDR Administration Endpoints.

To:

The mechanism for reporting will be via the Get Metrics endpoint.

and the following text and the list could be removed as it appears to be redundant:

The following information is to be reported:

  • Availability for current month
  • Availability for each of the previous twelve months
  • ...

@perlboy
Copy link

perlboy commented Sep 19, 2024

The Reporting Requirements section contains details that are out of sync with the latest Get Metrics v5 requirements.

I wonder why it needs to exist at all? The Standards define the Metrics endpoint, the Rules permit ACCC to require it, how is this an NFR?

@nils-work
Copy link
Member

In the Holder of Key Mechanism section, the 'section 3' link is broken -

[MTLS] HoK allows issued tokens to be bound to a client certificate as specified in section 3 of [MTLS].

@nils-work
Copy link
Member

Hi @perlboy
Re:

In https://consumerdatastandardsaustralia.github.io/standards/#cdr-dynamic-client-registration-api_schemas_tocSclientregistration it states:

Predefined error code as described in section 3.3 OIDC Dynamic Client Registration

But the overriding specification for DCR is RFC7591 and the CTS is now enforcing these error codes explicitly, which is noteworthy because 3.3 OIDC Dynamic Client Registration explicitly states "Other error codes MAY also be used."

Are you saying that the CTS only allows the two codes stated in Client Registration Error Response (invalid_redirect_uri, invalid_client_metadata) and not the four stated in RFC7591 and the Standards, or that it doesn't allow "Other error codes" outside those four?

@perlboy
Copy link

perlboy commented Oct 1, 2024

@nils-work that is correct, CTS will fail if a deliberately broken DCR does not return one of these errors. In the scenario we found our system was returning (right or wrong, we've now aligned to CTS), server_error because it was an unexpected condition that a DCR would be received where it would be a signed payload with a valid ACCC SSA for a kid that is not present at the JWKS endpoint.

image

To clarify, the "invalid SSA" the CTS references here is actually the signed POST (not the ACCC SSA) and was well formed and parseable but was signed by an absent kid.

@nils-work nils-work added this to the v1.33.0 milestone Oct 22, 2024
@nils-work
Copy link
Member

In the Security Endpoints > CDR Arrangement Revocation End Point section, there appears to be a duplicate line referring to the cdr_arrangement_jwt:

The proposal is to remove the duplicate first bullet and update these lines for clarity -

CDR Arrangement JWT method

The request MUST include the following parameters using the application/x-www-form-urlencoded format in the HTTP request entity-body:

  • cdr_arrangement_jwt: A signed JWT that includes the cdr_arrangement_id.
  • cdr_arrangement_jwt: A newly signed JWT with the following parameters in accordance with [JWT]:
    • cdr_arrangement_id: The ID of the arrangement that the client wants to revoke.

The cdr_arrangement_jwt SHOULD include all parameters in accordance with Data Holders calling Data Recipients using Self-Signed JWT Client Authentication.

To -

CDR Arrangement JWT method

The request MUST include the following parameter using the application/x-www-form-urlencoded format in the HTTP request entity-body:

  • cdr_arrangement_jwt: A newly signed JWT with the following parameters in accordance with [JWT]:
    • cdr_arrangement_id: The ID of the arrangement that the client wants to revoke.
    • This JWT SHOULD also include all parameters in accordance with Data Holders calling Data Recipients using Self-Signed JWT Client Authentication.

@nils-work
Copy link
Member

Review impact of #429 - Refer to components in Banking API spec.

@nils-work
Copy link
Member

The Energy specs don't have servers[...].url detail specified currently. This is inconsistent with other specs and means the 'Code samples' examples don't display a 'Host' property and a full URL.

The Energy Data Holder spec could be updated with https://data.holder.com.au/cds-au/v1 like other specs, or all specs could be updated to something such as https://dh.example.com/cds-au/v1

The Energy Secondary Data Holder spec could be updated with the actual hosts (including AEMO and MarketNet pre-prod and prod options) or a generic default such as https://sdh.example.com/cds-au/v1.

References to recipient.com.au could also be updated to adr.example.com where applicable.

Differences between TLS and MTLS endpoints could also be indicated, for example:

  • https://tls.dh.example.com/cds-au/v1
  • https://mtls.dh.example.com/cds-au/v1

This has been staged - https://github.com/ConsumerDataStandardsAustralia/standards-staging/pull/448/files

@nils-work
Copy link
Member

@HemangCDR
Copy link

HemangCDR commented Nov 13, 2024

In FDO table, the retirement statements for Get Generic Plan Detail v2 and Get Energy Account Detail v3 in FDO should have "from" instead of "by" :
image

also affects:
image
image

nils-work added a commit to ConsumerDataStandardsAustralia/standards-staging that referenced this issue Nov 15, 2024
@nils-work
Copy link
Member

In the Holder of Key Mechanism section, the 'section 3' link is broken -

[MTLS] HoK allows issued tokens to be bound to a client certificate as specified in section 3 of [MTLS].

This has been staged - ConsumerDataStandardsAustralia/standards-staging@10b081d

nils-work added a commit to ConsumerDataStandardsAustralia/standards-staging that referenced this issue Nov 15, 2024
@nils-work
Copy link
Member

In FDO table, the retirement statements for Get Generic Plan Detail v2 and Get Energy Account Detail v3 in FDO should have "from" instead of "by" : image

also affects: image image

This has been staged: ConsumerDataStandardsAustralia/standards-staging@6c19efc

@nils-work
Copy link
Member

This documentation fix will also be addressed as part of MI 21 - #675 - PAR/RFC9126 in Normative references appears twice.

@ACCC-CDR
Copy link

ACCC-CDR commented Dec 5, 2024

In https://consumerdatastandardsaustralia.github.io/standards/#cdr-dynamic-client-registration-api_schemas_tocSclientregistration it states:

Predefined error code as described in section 3.3 OIDC Dynamic Client Registration

But the overriding specification for DCR is RFC7591 and the CTS is now enforcing these error codes explicitly, which is noteworthy because 3.3 OIDC Dynamic Client Registration explicitly states "Other error codes MAY also be used."

Hi @perlboy,
In this instance, we consider the Consumer Data Standards as the source of truth and as such we interpret the CDS as stating:

  • The registration error property MUST be one of the four below values defined in the Registration Error Enumerated Values:
    • invalid_redirect_URI.
    • Invalid_client_metadata
    • Invalid_software_statement
    • Unapproved_software_statement
  • The RegistrationError error property is of type "enum" (an ‘enum’ as per the Common Field Types includes a ‘defined list of values’ and that ‘all possible values must be provided’.)

Whilst we acknowledge that '3.3 client registration error response for ‘OpenID Connect Dynamic Client Registration 1.0 incorporating errata set 2’
(https://openid.net/specs/openid-connect-registration-1_0.html#RegistrationError) defines that other error codes MAY also be used;

To pass this Invalid Software Statement Assertion step within CTS, one of the two error response types containing the below text, need to be returned:

  • Invalid_software_statement
  • Unapproved_software_statement

We appreciate your feedback regarding the wording of valid versus invalid SSA. We will also add that an invalid SSA in this case relates to a “SSA not signed by the register” and the team will include this additional information in the CTS Technical Guidance documentation, to help bring clarity for participants.

The aim of this CTS step is to ensure the participant is validating that the SSA is signed appropriately, which the participant should reject with an error, as described above.

Let us know if you have further feedback or queries.

Regards,
CDR Team

@perlboy
Copy link

perlboy commented Dec 5, 2024

Hi @perlboy, In this instance, we consider the Consumer Data Standards as the source of truth and as such we interpret the CDS as stating:

  • The registration error property MUST be one of the four below values defined in the Registration Error Enumerated Values:
    • invalid_redirect_URI.
    • Invalid_client_metadata
    • Invalid_software_statement
    • Unapproved_software_statement
  • The RegistrationError error property is of type "enum" (an ‘enum’ as per the Common Field Types includes a ‘defined list of values’ and that ‘all possible values must be provided’.)

Except the description of this field states:

Predefined error code as described in section 3.3 OIDC Dynamic Client Registration

This section defines only invalid_redirect_uri and invalid_client_metadata, the enum value defined in the Standards is neither defined in behaviour nor has any description. It's worth noting that OIDC DCR isn't a signed registration payload where as CDR is (ie. a signed JWT containing among other things the ACCC SSA JWT) so software statement error behaviour is not defined.

To pass this Invalid Software Statement Assertion step within CTS, one of the two error response types containing the below text, need to be returned:

  • Invalid_software_statement
  • Unapproved_software_statement

Neither of which are in 3.3 of OIDC DCR.

The aim of this CTS step is to ensure the participant is validating that the SSA is signed appropriately, which the participant should reject with an error, as described above.

Ummm, if this is the intent it does not appear to have achieved the objective. As I wrote before "the "invalid SSA" the CTS references here is actually the signed POST (not the ACCC SSA) and was well formed and parseable but was signed by an absent kid"

That is to say the CTS, as a Recipient, appears to be doing the following:

  1. Retrieving a valid SSA, signed by the Register
  2. Creating a DCR request as the CTS Recipient including the SSA from (1)
  3. Signing the DCR Request using a kid that does not exist at the CTS Recipient JWKS endpoint
  4. Expecting an invalid or unapproved software statement error in spite of the software statement supplied in (1) being valid.

The CTS team appears to be confusing the DCR POST to be a software statement - it is not - it is a [signed] Client Registration Request (in OIDC DCR parlance). This is why invalid_software_statement and unapproved_software_statement aren't listed as valid responses in OIDC DCR 3.3 because OIDC DCR doesn't deal in signed Client Registration Requests.

Further, invalid_client_metadata isn't appropriate because the description of it is The value of one of the Client Metadata fields is invalid and the server has rejected this request. That is to say, the client metadata isn't invalid, nothing in the metadata is invalid (including the ACCC SSA in software_statement), simply that the Client Registration Request is incorrectly signed. Since this is a uniquely CDR approach we can't really reference an actual standard hence the reversion to the generic OAuth2 server_error.

If the behaviour of the CTS instead did the following you would not only be successfully testing your intended use case but our implementation would function the way you expected:

  1. Conjur up a a structurally valid SSA signed by an unknown kid
  2. Create a DCR Request as the CTS Recipient
  3. Sign the DCR Request using the accepted JWKS endpoint
  4. Expect invalid or unapproved software statement

Regardless, clearly there's a misunderstanding of how things should be interpreted because the Standards clearly state error behaviours the CTS isn't supporting and they aren't documented sufficiently for an implementer to actually understand how it should be.

In the meantime it seems that the CTS has now forced a behaviour on Holders that doesn't actually make sense and in fact is the worst possible combination - Recipients will now receive the same error if it is believed Recipient or the ACCC incorrectly sign something - a condition that we've observed on both sides due to Holders having network issues accessing the Recipient or the ACCCs JWKS endpoint. If it had at least accepted invalid_client_metadata it would have been possible to differentiate.

nils-work added a commit to ConsumerDataStandardsAustralia/standards that referenced this issue Dec 18, 2024
* Created 1.32.0 branch base

* Staging #429 - Refactor Banking API spec

Addresses: ConsumerDataStandardsAustralia/standards-staging#429

* Create 1.33.0 branch base

* Updated refresh token expiry requirements related to Standards Maintenance issue #667

* Made links relative to avoid page refresh

Addresses: ConsumerDataStandardsAustralia/standards-staging#435 (comment)

* Added and updated servers in specs and examples

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#663 (comment)

* Link to specific comment

* Remove scheme from host field

* Clarified 'CDR Arrangement JWT method' details

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#663 (comment)

* Updated Reporting Requirements section

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#663 (comment)

* Replaced 'must' with 'MUST' in some headers

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#473 (comment)

* Applied consistent styling

Addresses: ConsumerDataStandardsAustralia/standards-staging#442

* Key word styling

Addresses: ConsumerDataStandardsAustralia/standards-staging#443

* Move note to correct section

* Corrected typos, updated styling

Addresses: ConsumerDataStandardsAustralia/standards-staging#431

* Staging #429 - Refactor Banking API spec

Addresses: ConsumerDataStandardsAustralia/standards-staging#429

* Update MTLS section 3 link

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#663 (comment)

* Clarify retirement date statements

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#663 (comment)

* Updated Obligation Date Schedule

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#661

* Clarified transaction security requirements

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#654

* Updated field descriptions

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#655

* Updated CX Standards

Addresses: #350

* Updated Normative References

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#675

* Tidied table formatting

* Get Product Detail v5 with LVR constraints

Addresses: ConsumerDataStandardsAustralia/standards-maintenance#657

* Rename file to prevent generated version

Addresses: ConsumerDataStandardsAustralia/standards-staging#463

* Adjusted values to align format

* Corrected Date Schedule

* Standards Maintenance 664: Support for additional NPP service overlays and all versions

* Added diff for FDO table

* Change CDS links to DSB

Addresses: ConsumerDataStandardsAustralia/standards-staging#468

* Closed list items in unordered list

* Minor corrections

* Updates

* Minor updates

- customer > consumer
- obligation sequence

* Deprecated OiDC Hybrid Flow with retirement date set for May 12th 2025

* Add condition

* Updated diff comment for OIDC hybrid flow to indicate it is deprecated

* Update get-transaction-detail-v1.html.md

* Apply dates, logo and correct merge differences

* Rebuild

* Rebuild specs

* Minor corrections

* Update obligation table

---------

Co-authored-by: Mark Verstege <[email protected]>
@nils-work nils-work moved this from In Progress: Staging to Awaiting Chair Approval in Data Standards Maintenance Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Non-breaking change A change that is not expected to result in a new endpoint version.
Projects
Status: Awaiting Chair Approval
Development

No branches or pull requests

5 participants