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

Update READMEs, docs, add how-tos section, first how-to for DIDs #607

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

decentralgabe
Copy link
Member

Address part of #606

@codecov-commenter
Copy link

codecov-commenter commented Jul 28, 2023

Codecov Report

Merging #607 (ef50476) into main (b3ad6d1) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #607      +/-   ##
==========================================
+ Coverage   26.97%   27.03%   +0.05%     
==========================================
  Files          48       48              
  Lines        5860     5830      -30     
==========================================
- Hits         1581     1576       -5     
+ Misses       3995     3971      -24     
+ Partials      284      283       -1     
Files Changed Coverage Δ
pkg/server/server.go 67.10% <100.00%> (+5.86%) ⬆️

@decentralgabe decentralgabe changed the title Update READMEs, docs, add how-tos Update READMEs, docs, add how-tos section, first how-to for DIDs Jul 28, 2023
@decentralgabe decentralgabe marked this pull request as ready for review July 28, 2023 04:26
@michaelneale
Copy link
Contributor

SMASH THE LIKE BUTTON SO HARD I BROKE MY KEYBOARD
image

@michaelneale
Copy link
Contributor

@decentralgabe is this a WIP? If so, can we have a flow on the REAME with curl which issues a credential for the simplest schema? (as one block example) - that would be wonderful

| [Issuing a Credential](https://github.com/TBD54566975/ssi-service/blob/main/doc/howto/credential.md) | Get started with credential issuance functionality |
| [Verify a Credential](https://github.com/TBD54566975/ssi-service/blob/main/doc/howto/verification.md) | Get started with credential verification functionality |
| [Revoke/Suspend a Credential](https://github.com/TBD54566975/ssi-service/blob/main/doc/howto/status.md) | Get started with credential status functionality |
| [[TODO] Requesting and Verifying Credentials with Presentation Exchange](https://github.com/TBD54566975/ssi-service/issues/606) | Get started with Presentation Exchange functionality |
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -0,0 +1,122 @@
# How To: Create a DID

## Background
Copy link
Contributor

Choose a reason for hiding this comment

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

Would suggest a TL;DR section with a oneliner curl command. WDYT?

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 below

DIDs are a core component of SSI (Self-Sovereign Identity) systems. DIDs are specified according to "methods" which
define how the DID is created and how it can be used. A list of existing [DID methods can be found here](https://www.w3.org/TR/did-spec-registries/#did-methods).

Importantly, DIDs in the SSI Service are _fully custodial_. This means the private keys associated with DID Documents are managed by the service and never leave its boundaries. For some DID methods, such as `did:web` it's possible to add multiple keys to the DID document, and it's possible for these additional keys to be added outside the service. This is a more advanced concept that the service may support in the future.
Copy link
Contributor

Choose a reason for hiding this comment

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

!! super important.

license:
name: Apache 2.0
url: http://www.apache.org/licenses/LICENSE-2.0.html
title: SSI Service API
version: '{{.SVN}}'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should PIN this in source code, and remove from the configuration file. What's the purposed of having it be configurable? We have no logic that's making decisions off of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

agreed

Copy link
Member Author

Choose a reason for hiding this comment

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

we have this issue to track #502 don't want to overload this change set

engine.GET(SwaggerPrefix, ginswagger.WrapHandler(swaggerfiles.Handler, ginswagger.URL("/swagger.yaml")))
}
engine.StaticFile("swagger.yaml", "./doc/swagger.yaml")
engine.GET(SwaggerPrefix, ginswagger.WrapHandler(swaggerfiles.Handler, ginswagger.URL("/swagger.yaml")))
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this makes it difficult to specify the server urls dynamically. This is important to actually have a nice swagger config.

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, but doing it with a template file is not a great solution because it leaves ugly template files to render on the website

using swaggo's default override system as we had before doesn't have this issue, so I will re-add that

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as there is a way of handling that use case, then this lgtm.

Copy link
Contributor

@andresuribe87 andresuribe87 left a comment

Choose a reason for hiding this comment

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

Approving subject to having a way of specifying server urls in the swagger file.

@decentralgabe
Copy link
Member Author

@michaelneale WIP yes, will take the VC stuff in a follow up PR didn't want this to get too big!

@decentralgabe decentralgabe merged commit b57a656 into main Jul 28, 2023
6 checks passed
@decentralgabe decentralgabe deleted the update-docs branch July 28, 2023 17:27
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.

4 participants