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

status codes for discovery client #3513

Merged
merged 3 commits into from
Oct 25, 2024

Conversation

gerardsn
Copy link
Member

@gerardsn gerardsn commented Oct 23, 2024

closes #3502

PR contains

  • add clientRegistrationManager.getServiceAndSubject to check for service and subject
  • Add 404 for unknown subjectID and serviceID
  • Add 202 on DELETE as documented in OAPI spec
  • Log ignored errors
  • cleanup api/server#client.New

discovery/client.go Outdated Show resolved Hide resolved
@@ -116,7 +110,7 @@ func (r *defaultClientRegistrationManager) activate(ctx context.Context, service
subjectDIDs = subjectDIDs[:j]

if len(subjectDIDs) == 0 {
return fmt.Errorf("%w: %w for %s", ErrPresentationRegistrationFailed, didsubject.ErrSubjectNotFound, subjectID)
return fmt.Errorf("%w: %w for %s", ErrPresentationRegistrationFailed, ErrDIDMethodsNotSupported, subjectID)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to change the error to something like: "no active DIDs found for subject"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check here is that the subject has one or more DIDs that are in the accepted DID methods (error removed above) AND that at least one of the DIDs is active. The second part is internal bookkeeping that imo should not be in the response.

this way it is consistent with deactivate.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a new error for this with message subject has no (active) DIDs matching the service

@gerardsn gerardsn merged commit 9296c03 into master Oct 25, 2024
8 of 9 checks passed
@gerardsn gerardsn deleted the iss-3502/discovery-service-api-status-codes branch October 25, 2024 08:04
rolandgroen added a commit that referenced this pull request Nov 1, 2024
* master: (72 commits)
  PKI add ValidateStrict (#3531)
  PEX: Return capture group for matched patterns (#3526)
  Schedule CodeQL twice a week (#3525)
  change cron schedule (#3524)
  Add gh action for CodeQL schedule (#3523)
  Bump github.com/lestrrat-go/jwx/v2 from 2.1.1 to 2.1.2 (#3520)
  docs: v6 release date (#3519)
  status codes for discovery client (#3513)
  Require SQL connection string in strictmode (#3517)
  Fix duplicate discovery results (#3515)
  Bump github.com/chromedp/chromedp from 0.11.0 to 0.11.1 (#3514)
  fix duplicate search results for wildcard param (#3512)
  Bump github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys (#3511)
  remove network migration and optimize network event retry (#3510)
  secure outgoing http client with max connections (#3508)
  make gen-mocks (#3509)
  Bump github.com/Azure/azure-sdk-for-go/sdk/azcore from 1.15.0 to 1.16.0 (#3506)
  fix invalid keyReference migration objects (#3504)
  Bump go.uber.org/mock from 0.4.0 to 0.5.0 (#3507)
  Bump github.com/nats-io/nats-server/v2 from 2.10.21 to 2.10.22 (#3505)
  ...

# Conflicts:
#	vcr/test.go
#	vdr/legacy_integration_test.go
#	vdr/vdr.go
#	vdr/vdr_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discovery Service: check API responses for correct status codes
2 participants