Skip to content
This repository has been archived by the owner on Nov 7, 2023. It is now read-only.

express-did-auth - Implement personal_sign user signature #72

Merged
merged 9 commits into from
Nov 25, 2020
Merged

express-did-auth - Implement personal_sign user signature #72

merged 9 commits into from
Nov 25, 2020

Conversation

ilanolkies
Copy link
Contributor

@ilanolkies ilanolkies commented Nov 16, 2020 •
edited
Loading

This PR enforces the user to sign using personal_sign instead of JWTs. Reference: rsksmart/rif-identity-docs#28

Status of the implementation:

  • Implement a happy path using personal_sign
  • Remove skipped tests
  • Correct tests to use new signature kind
  • Validate all error messages are still correct
  • Clean unused code - removed code will keep commented until this phase

@ilanolkies ilanolkies added the bug Something isn't working label Nov 16, 2020
@ilanolkies ilanolkies added this to the Express DID Auth v0.1 milestone Nov 16, 2020
@ilanolkies ilanolkies added this to In progress in RIF Identity Q4 20' via automation Nov 16, 2020
@ilanolkies ilanolkies changed the title express-did-auth - Implement happy path using personal_sign express-did-auth - Implement personal_sign for signatures Nov 16, 2020
@ilanolkies ilanolkies changed the title express-did-auth - Implement personal_sign for signatures express-did-auth - Implement personal_sign user signature Nov 16, 2020
@ilanolkies ilanolkies marked this pull request as ready for review November 17, 2020 18:29
@javiesses javiesses mentioned this pull request Nov 19, 2020
Closed
javiesses
javiesses reviewed Nov 20, 2020
View reviewed changes
Copy link
Contributor

@javiesses javiesses left a comment

Choose a reason for hiding this comment

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

Like it! Minor changes requested

packages/express-did-auth/test/authentication-factory.test.ts Outdated Show resolved Hide resolved
packages/express-did-auth/test/authentication-factory.test.ts Outdated Show resolved Hide resolved
packages/express-did-auth/test/setup-app.cookies.integration.test.ts Outdated Show resolved Hide resolved
packages/express-did-auth/test/setup-app.cookies.integration.test.ts Outdated Show resolved Hide resolved
ilanolkies and others added 4 commits November 20, 2020 15:58
@ilanolkies @javiesses
Update packages/express-did-auth/test/authentication-factory.test.ts
ef1d29a 
Co-authored-by: Javier Esses <javiesses@hotmail.com>
@ilanolkies @javiesses
Update packages/express-did-auth/test/authentication-factory.test.ts
894748d 
Co-authored-by: Javier Esses <javiesses@hotmail.com>
@ilanolkies @javiesses
Apply suggestions from code review
eade23e 
Co-authored-by: Javier Esses <javiesses@hotmail.com>
@ilanolkies @javiesses
Update packages/express-did-auth/test/authentication-factory.test.ts
da4efa9 
Co-authored-by: Javier Esses <javiesses@hotmail.com>
javiesses
javiesses approved these changes Nov 20, 2020
View reviewed changes
Copy link
Contributor

@javiesses javiesses left a comment

Choose a reason for hiding this comment

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

LGTM

RIF Identity Q4 20' automation moved this from In progress to Reviewer approved Nov 20, 2020
@ilanolkies ilanolkies merged commit 2ff2f4e into develop Nov 25, 2020
RIF Identity Q4 20' automation moved this from Reviewer approved to Done Nov 25, 2020
@delete-merged-branch delete-merged-branch bot deleted the did-auth-personal-sign branch November 25, 2020 15:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@javiesses javiesses javiesses approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
RIF Identity Q4 20'
  
Done
Milestone
Express DID Auth v0.1
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ilanolkies @javiesses