camaraproject / Commonalities

Repository to describe, develop, document and test the common guidelines and assets for CAMARA APIs
Apache License 2.0
9 stars 24 forks source link

error_model_alignment #213

Closed PedroDiez closed 3 weeks ago

PedroDiez commented 1 month ago

What type of PR is this?

Add one of the following kinds:

What this PR does / why we need it:

This PR covers mainly 3 topics:

Documentation Impacted:

Which issue(s) this PR fixes:

Fixes #127 Fixes #128 Fixes #162 Fixes #172 Fixes #180

Special notes for reviewers:

Regarding API design guidelines, section 6.2 could also be documented in a separate file (.md for instance and refer from that section). I preferred to explicitly reflect to be easy to get the aim of the section.

@rartych regarding Issue #180 in case you were thinking in additional points I can just take out (Fixes #180) and only reference in the (#### What this PR does / why we need it:) as partial fixing

Changelog input

 - Clarify Error properties are required
 - Error model for device Identifier
 - Added clarification about AuthN/AuthZ standardized flows follow their own Error Response format
 - Error model alignment

Additional documentation

Not apply

rartych commented 1 month ago

To tackle #128 and #172 and similar questions I suggest to add paragraph like:

When standardized authentication flows are used - cf. 10.2 Security Implementation the format and content of Error Response should follow relevant standards.

It can be in error format definition or particularly for 401 Status.

jlurien commented 1 month ago

I have linked another related discussion in QoD (https://github.com/camaraproject/QualityOnDemand/pull/297) to define the status and code error for situations when a request is rejected due to exceeding a business quota limit. A new code QUOTA_EXCEEDED makes sense, but status could be either 429 or 403. The first being more related to rate limiting and the second a more generic "forbidden due to business logic".

I will align the PR in QoD with the decision here. cc @PedroDiez

PedroDiez commented 1 month ago

Actions covered in commit 4611515ce7eedb10c961d14b1ccb3966dcdc447b

PedroDiez commented 1 month ago

Actions covered in commit 082f5e52ddec8920432fe12d97cbce62e4cb5fb3

PedroDiez commented 1 month ago

05/JUN: Addressed first round of reviews

Pending point: