ConsumerDataStandardsAustralia / standards-maintenance

This repository houses the interactions, consultations and work management to support the maintenance of baselined components of the Consumer Data Right API Standards and Information Security profile.
41 stars 9 forks source link

Maintenance Iteration 14 Holistic Feedback #565

Closed CDR-API-Stream closed 1 year ago

CDR-API-Stream commented 1 year ago

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 14 and the DSB will review them. If a suggestion is a material change a dedicated CR will be raised.

nils-work commented 1 year ago

Three documentation fixes -

In the Certificate Management section, the Function column of the Server Certificate(s) row of the Issued by the Register CA for Data Recipients table, it states -

Secures the following: - Revocation endpoint - CDR Arrangement Management endpoint - JWKS endpoint

  1. This should be updated to merge the 'revocation' and 'arrangement management' endpoint references -

    Secures the following: - CDR Arrangement Revocation endpoint - JWKS endpoint

  2. The Notes column of the same row has a typo; remove an -

    ...with ~an~ the Register CA issued certificate...

  3. The line immediately below that table has a typo; the the Register. This may be more correct as the ACCC... -

    ...issues and manages certificates to CDR participants as directed by the ~the Register~ACCC in its capacity as the CDR Registrar.

nils-work commented 1 year ago

Consider the following documentation updates to include a link in the description of each Get Metrics schema to the related section of the standards -

AvailabilityMetrics

Percentage availability of the CDR platform over time. For details refer to Availability Requirements.

PerformanceMetrics

Percentage of calls within the performance thresholds. For details refer to Performance Requirements.

InvocationMetricsV2

Number of API calls in each performance tier over time. For details refer to Performance Requirements.

AverageResponseMetricsV2

Average response time in seconds, at millisecond resolution, within each performance tier. For details refer to Performance Requirements.

SessionCountMetrics

Session counts over time. Note that a session is defined as the provisioning of an Access Token. For details refer to Tokens.

ErrorMetrics

Number of calls resulting in error due to server execution over time. For details refer to HTTP Response Codes.

RejectionMetricsV2

Number of calls rejected due to traffic thresholds over time. For details refer to Traffic Thresholds.

For the following items in the ResponseMetricsListV3 table -

» customerCount

Number of customers with active authorisations at the time of the call. For details refer to CDR Federation.

» recipientCount

Number of Data Recipient Software Products with active authorisations at the time of the call. For details refer to CDR Federation.

For the following items in the SecondaryHolderMetrics table -

errors

Number of calls resulting in error due to server execution over time. For details refer to HTTP Response Codes.

rejections It appears the current description may be incorrect. rejection due to server execution appears to be a drafting error copied from the errors property. Rejections are defined as being due to traffic thresholds, but as thresholds are not currently stated for Secondary Data Holders (SDH), this could instead refer to the exemptions section, which may convey the responses expected from the SDH-

Number of calls rejected due to traffic thresholds over time. For details refer to Exemptions To Protect Service.

nils-work commented 1 year ago

Typo in RejectionMetricsV2

Rejection counts for all ~uauthenticated~ unauthenticated end points

nils-work commented 1 year ago

The Test Documentation link at the bottom of the left navigation column is incorrect, it should point to - https://consumerdatastandardsaustralia.github.io/standards-testing/

ShaneDoolanFZ commented 1 year ago

Text correction

In the BankingAccountDetailV2 section there is a copy/paste error in the lendingRates description.

Fully described deposit rates for this account based on the equivalent structure in Product Reference

perlboy commented 1 year ago

1.21.0 regressed and readded open-status query parameter on Get Energy Account Detail. This was originally marked as resolved in https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/530#issuecomment-1246151526.

The merge hell of standards and standards-maintenance makes it hard to see through the web of cross repo merges so I'm not going to go down that rabbit hole. It does raise the question as to whether there are other regressions between 1.19.0 and 1.21.0 though.

Nonetheless as 1.21.0 is now the binding standard please urgently reapply the change as implementers outside the big 3 energy retailers are now in active scoping.

JamesMBligh commented 1 year ago

This will be fixed today in v1.22.0 (the maintenance iteration 13 change)

tinagroark commented 1 year ago

I also note that CDS references two versions of the OAuth 2.0 Pushed Authorization Requests standard

should references to the draft version be changed to rfc9126?

nils-work commented 1 year ago

Consider incorporating some of the documentation fixes described in https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/527#issuecomment-1398104114

nils-work commented 1 year ago

Fix the typo in the first line of the Session Requirements section -

A The expiry time of a unique session should be set according to the statements included in the Security Profile.

nils-work commented 1 year ago

There appears to be a typo in the scope name - Admin Metadata Update Data. This scope provides the ability to provide a notification only, and data is not returned by the associated endpoint.

nils-work commented 1 year ago

It has been suggested that the two "Valid Examples" for the RateString Common Field Type could be confusing for participants.

The definition states -

A string representing a percentage (e.g. an interest rate). A rate of 100% would be represented by the value 1.0 and a rate of -100% by -1.0

Although values higher than 1 and lower than -1 could be valid, the third example may be misinterpreted as meaning ~12.34%

The suggestion is to simplify the third and fourth examples - Current example Equates to Proposed example Equates to
“0.05” 5% (no change) 5%
“-0.05” -5% (no change) -5%
“12.3456789” 1234.56789% "0.123456789" 12.3456789%
“-99.123456789123” -9912.32456789123% “-0.99123456789” -99.123456789%
nils-work commented 1 year ago

There is a typo in the pseudo code associated with an 'Invalid Page Size' error in the Error Codes section, under 400 Bad Request Errors: page_size > 1000

Should be - page-size > 1000

nils-work commented 1 year ago

There is a typo in the description of the period parameter of the Get Metrics endpoint.

Where it states - Values can be CURRENT (meaning metrics for current day)

It should state - Values can be CURRENT (meaning metrics for current day or month)

nils-work commented 1 year ago

Additional clarity could be provided to state where CORS (Cross-Origin Resource Sharing) settings are not required for some unauthenticated endpoints that are infosec-related, rather than being 'resource' endpoints.

More detail is provided in this issue - Clarify where CORS is not required #266

nils-work commented 1 year ago

To clarify the application of the tiers in the Performance Requirements section, with respect to the differences between -

consider the following documentation changes -

  1. In the Unauthenticated row, change this text:

    1. All Unauthenticated end points to All unauthenticated endpoints (lower case 'u')
  2. In the High Priority row, change these lines:

    1. The following Unauthenticated end points: to The following unauthenticated resource endpoints:

    2. Customer Present calls to the following end points: to Customer Present calls to the following authenticated resource endpoints:

  3. In the Low Priority row, change this text:

    1. Customer Present calls to the following end points: to Customer Present calls to the following authenticated resource endpoints:
  4. In the Unattended row, change these lines:

    1. High Priority Authenticated end points to High Priority authenticated resource endpoints

    2. Low Priority Authenticated end points to Low Priority authenticated resource endpoints

  5. In the Large Payload, Secondary Request and Large Secondary Request rows, change this text:

    1. calls to the following end points to calls to the following resource endpoints

There also appear to be a few typos in this section, in the Energy endpoint names -

nils-work commented 1 year ago

A clarification of the PENSION_RECIPIENT eligibilityType (and discountEligibilityType, which has similar wording) as was suggested in this comment could be made, as reiterated by published guidance. This change could resolve issue #135, by aligning these types to other values (STAFF, STUDENT, MIN_AGE, MIN_TURNOVER)

The change is to insert 'Only' at the start of the descriptions -

Field Value Description
eligibilityType PENSION_RECIPIENT Only a recipient of a government pension may apply for the product
discountEligibilityType PENSION_RECIPIENT Only a recipient of a government pension may receive the discount
CDR-API-Stream commented 1 year ago

Three documentation fixes -

In the Certificate Management section, the Function column of the Server Certificate(s) row of the Issued by the Register CA for Data Recipients table, it states -

Secures the following:

  • Revocation endpoint
  • CDR Arrangement Management endpoint
  • JWKS endpoint
  1. This should be updated to merge the 'revocation' and 'arrangement management' endpoint references -

Secures the following:

  • CDR Arrangement Revocation endpoint
  • JWKS endpoint
  1. The Notes column of the same row has a typo; remove an -

...with ~an~ the Register CA issued certificate...

  1. The line immediately below that table has a typo; the the Register. This may be more correct as the ACCC... -

...issues and manages certificates to CDR participants as directed by the ~the Register~ACCC in its capacity as the CDR Registrar.

This issue has been fixed. It is staged for review here https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/a32e3e2a9a266339263b8d710457d61846abea32

CDR-API-Stream commented 1 year ago

Typo in RejectionMetricsV2

Rejection counts for all ~uauthenticated~ unauthenticated end points

This issue has been fixed. It is staged for review here https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/ff6ea40f089587d720d72bc2689af9579bd836bc

CDR-API-Stream commented 1 year ago

Text correction

In the BankingAccountDetailV2 section there is a copy/paste error in the lendingRates description.

Fully described deposit rates for this account based on the equivalent structure in Product Reference

This issue has been fixed for BankingAccountDetailV3 as per the current standards. It is staged for review here https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/a59e54ac4b442f08fd808e3372e00d6d0854ddf6

CDR-API-Stream commented 1 year ago

The following issues have been fixed and staged for review here https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/8e1a0b920f6372329ff5747ffbcc69e6fbe63341:

Refer to comment for change details.

CDR-API-Stream commented 1 year ago

There is a typo in the description of the period parameter of the Get Metrics endpoint.

Where it states - Values can be CURRENT (meaning metrics for current day)

It should state - Values can be CURRENT (meaning metrics for current day or month)

This issue has been addressed by updating the description of period parameter with the following text:

It is staged and available for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/4e396088f8160a86f19206b58d68cc768dcbc93b

CDR-API-Stream commented 1 year ago

A clarification of the PENSION_RECIPIENT eligibilityType (and discountEligibilityType, which has similar wording) as was suggested in this comment could be made, as reiterated by published guidance. This change could resolve issue #135, by aligning these types to other values (STAFF, STUDENT, MIN_AGE, MIN_TURNOVER)

The change is to insert 'Only' at the start of the descriptions -

Field Value Description eligibilityType PENSION_RECIPIENT Only a recipient of a government pension may apply for the product discountEligibilityType PENSION_RECIPIENT Only a recipient of a government pension may receive the discount

This issue has been fixed and staged for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/c0c81f3b121050c0db5ea9e797c4cd772a5ff0d8

CDR-API-Stream commented 1 year ago

Additional clarity could be provided to state where CORS (Cross-Origin Resource Sharing) settings are not required for some unauthenticated endpoints that are infosec-related, rather than being 'resource' endpoints.

More detail is provided in this issue - Clarify where CORS is not required #266

This issue has been fixed and staged for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/a325e68138924bb074916d28581c7aeeae23e778

CDR-API-Stream commented 1 year ago

To clarify the application of the tiers in the Performance Requirements section, with respect to the differences between -

  • InfoSec and resource endpoints,
  • authenticated and unauthenticated endpoints,
  • Customer Present and Unattended calls,

consider the following documentation changes -

  1. In the Unauthenticated row, change this text:

    1. All Unauthenticated end points to All unauthenticated endpoints (lower case 'u')
  2. In the High Priority row, change these lines:

    1. The following Unauthenticated end points: to The following unauthenticated resource endpoints:
    2. Customer Present calls to the following end points: to Customer Present calls to the following authenticated resource endpoints:
  3. In the Low Priority row, change this text:

    1. Customer Present calls to the following end points: to Customer Present calls to the following authenticated resource endpoints:
  4. In the Unattended row, change these lines:

    1. High Priority Authenticated end points to High Priority authenticated resource endpoints
    2. Low Priority Authenticated end points to Low Priority authenticated resource endpoints
  5. In the Large Payload, Secondary Request and Large Secondary Request rows, change this text:

    1. calls to the following end points to calls to the following resource endpoints

There also appear to be a few typos in this section, in the Energy endpoint names -

  • Get Accounts should be Get Energy Accounts
  • Get Account Detail should be Get Energy Account Detail
  • Get Balance For Account should be Get Balance For Energy Account
  • Get Bulk Balances should be Get Bulk Balances for Energy
  • Get Balances For Specific Accounts should be Get Balances For Specific Energy Accounts

This issue has been fixed by explicitly listing APIs that belong in the "Unattended" section. This aligns it with the other categories. The incorrect energy API names have also been corrected. The staged changes can be viewed here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/3972bcbb2320de9c180ef68d858cb742ee817815

CDR-API-Stream commented 1 year ago

Consider the following documentation updates to include a link in the description of each Get Metrics schema to the related section of the standards -

AvailabilityMetrics

Percentage availability of the CDR platform over time. For details refer to Availability Requirements.

PerformanceMetrics

Percentage of calls within the performance thresholds. For details refer to Performance Requirements.

InvocationMetricsV2

Number of API calls in each performance tier over time. For details refer to Performance Requirements.

AverageResponseMetricsV2

Average response time in seconds, at millisecond resolution, within each performance tier. For details refer to Performance Requirements.

SessionCountMetrics

Session counts over time. Note that a session is defined as the provisioning of an Access Token. For details refer to Tokens.

ErrorMetrics

Number of calls resulting in error due to server execution over time. For details refer to HTTP Response Codes.

RejectionMetricsV2

Number of calls rejected due to traffic thresholds over time. For details refer to Traffic Thresholds.

For the following items in the ResponseMetricsListV3 table -

» customerCount

Number of customers with active authorisations at the time of the call. For details refer to CDR Federation.

» recipientCount

Number of Data Recipient Software Products with active authorisations at the time of the call. For details refer to CDR Federation.

For the following items in the SecondaryHolderMetrics table -

errors

Number of calls resulting in error due to server execution over time. For details refer to HTTP Response Codes.

The proposed links to documentation sections were not included in this change as they may become difficult to maintain and are less useful when consumed outside that Standards in the affected JSON/YAML schema files.


rejections It appears the current description may be incorrect. rejection due to server execution appears to be a drafting error copied from the errors property. Rejections are defined as being due to traffic thresholds, but as thresholds are not currently stated for Secondary Data Holders (SDH), this could instead refer to the exemptions section, which may convey the responses expected from the SDH-

Number of calls rejected due to traffic thresholds over time. For details refer to Exemptions To Protect Service.

The above change has been fixed (excluding the link) and staged for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/c8f34626d37123b94e377f1e61e96efe0d19db9c

CDR-API-Stream commented 1 year ago

It has been suggested that the two "Valid Examples" for the RateString Common Field Type could be confusing for participants.

The definition states -

A string representing a percentage (e.g. an interest rate). A rate of 100% would be represented by the value 1.0 and a rate of -100% by -1.0

Although values higher than 1 and lower than -1 could be valid, the third example may be misinterpreted as meaning ~12.34%

The suggestion is to simplify the third and fourth examples -

Current example Equates to Proposed example Equates to “0.05” 5% (no change) 5% “-0.05” -5% (no change) -5% “12.3456789” 1234.56789% "0.123456789" 12.3456789% “-99.123456789123” -9912.32456789123% “-0.99123456789” -99.123456789%

This issue has been addressed by adding specific % values represented by the current RateString examples. It is staged and available for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/5025c020580e3fac491f6a1228b3533be59d40fc

CDR-API-Stream commented 1 year ago

These items were addressed in version 1.24.0 of the standards as part of MI14: https://github.com/ConsumerDataStandardsAustralia/standards/issues/281#issuecomment-1533902679