demo of full authorization flow #147
Conversation
|
@decentralgabe can you take a look at this and let me know if at least the flow is correct and there's nothing wrong conceptually with the demonstration? |
|
@decentralgabe yea...we can break it up into multiple files. Good idea. Thanks for the comments. |
|
I need to eventually bring this back to life. Sorry, been caught up with other things. |
|
Based on #164 will move over to use_cases. |
Codecov Report
@@ Coverage Diff @@
## main #147 +/- ##
==========================================
- Coverage 55.72% 55.10% -0.63%
==========================================
Files 38 40 +2
Lines 4163 4368 +205
==========================================
+ Hits 2320 2407 +87
- Misses 1449 1565 +116
- Partials 394 396 +2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@decentralgabe I think this example will work. I've incorporated the feedback provided. There's things that I would like to improve, in later PR's, but for now if possible I'd like to just get this example in, and improve the example later. It's been 29 days this PR's been open now 🙄 ( mia culpa. it just slid off my radar ) Can you take a look when you have a chance, and we can discuss what changes would be needed for a merge? I'd like to limit future work on it, but I don't want to scrap this work either as it accomplishes I think, it's primary goal of being a simple example to demonstrate a full issuance and verification flow. I think this with the apartment use case: https://github.com/TBD54566975/ssi-sdk/tree/main/example/use_cases/apartment_application are reasonable reference points for a beginner.
|
|
@decentralgabe please check out the comment above re: my position on this PR. |
example/use_cases/employer_university_flow/employer_university_flow/issuer.go
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| jwk, err := cryptosuite.GenerateJSONWebKey2020(cryptosuite.OKP, cryptosuite.Ed25519) | ||
| if err != nil { |
There was a problem hiding this comment.
curious why not util.HandleExampleError here?
There was a problem hiding this comment.
ah. this is because I don't handle the errors here. I handle errors in a single place. This just propagates the error up to the parent function. Basically, propagate the errors until the end, and then determine handling methods. This is from my understanding, best practice:
https://earthly.dev/blog/golang-errors/
The util will kick an exit of a program, which may want to be changed later or handled differently on for some reason, so we want to push the error upstream and handle it there not downstream.
| presentationData, err := emp.MakePresentationData("test-id", "id-1") | ||
| util.HandleExampleError(err, "failed to create pd") | ||
| if dat, err := json.Marshal(presentationData); err == nil { | ||
| logrus.Debugf("Presentation Data:\n%v", string(dat)) |
There was a problem hiding this comment.
if there's an error here, the program will continue?
There was a problem hiding this comment.
yea...the error would be contained to marshaling data. 1) It shouldn't ever really happen ( a failure to marshal ) but I see it as a design choice not a bug if it decides to not exit. There could be cases, where json marshaling fails for some reason.
I'll change it to exit but wanted to note the above. Thanks for the feedback!
| return nil, err | ||
| } | ||
|
|
||
| if dat, err := json.Marshal(knownCred); err == nil { |
There was a problem hiding this comment.
what happens if this is an error? does the "VC issued" line still show?
There was a problem hiding this comment.
I was thinking we just ignore errors that are logging/marshaling json errors, but we can handle them. I don't think they should happen often if at all.
| return requestJWTBytes, signer, err | ||
| } | ||
|
|
||
| func MakePresentationRequest(jwk cryptosuite.JSONWebKey2020, presentationData exchange.PresentationDefinition, targetId string) (pr []byte, signer *cryptosuite.JSONWebKeySigner, err error) { |
There was a problem hiding this comment.
What is the point of this?
There was a problem hiding this comment.
I'm assuming the question is why private and public mirror functions. So the idea was originally keeping everything private. Then I realized that I wanted to make some public, but I didn't want to refactor the private function calls. I've also seen this done sometimes, in production cases, to keep separation of concerns. Private for functionality, then a wrapper to expose it. If you need to hide the function again, it's trivial and doesn't require as much of a refactor.
That being said: The style of private functions with an external wrapper is probably not useful for this demo, or adds another layer of thought, so I've adjusted things. Thanks for the feedback!
| } | ||
|
|
||
| CustomWriter.WriteNote(fmt.Sprintf("DID for holder is: %s", didStr)) | ||
| s.AddPrivateKey("main", privKey) |
There was a problem hiding this comment.
nit: what do you think about indexing the keys with integers? this could be simpler...but i also see some benefit in having named keys...
There was a problem hiding this comment.
I think we could definitely expand the wallet or do some other types of indexing. Another project I'm working on does this.
To me though, there's another scope of work around making good wallets, and the scope of this PR is to make just a wallet that is simple in functionality and gives the general "idea". As long as it works, and can educate, at least IMO, it's sufficient for a PR, with a possible follow up on an issue about doing an adjustment.
But ultimately your call. If you want, we can explore different wallet styles in the PR, but it might result in some extra time required for acceptance.
There was a problem hiding this comment.
agree it's for a follow up
|
@decentralgabe thanks for the review! Hopefully addressed the comments/concerns. Made a few notes about some thoughts. Lmk if there's anything else. Looking forward to getting this in. |
a sample demo with a full authorization flow. This is moved from #123. In this, 3 actors are presented:
The situation is simple: A student graduates from Example University, and Employer now wants to hire the student. This flow outlines how the VC flow works, between the holder, issuer, and verifier. Below is an example of the flow.
There is a basic authorization check later, which checks if the employer is OK with the student's VC claims. This is intended to be an explanatory demo, not a production use case.