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

feature(auth): add SRP authentication #25

Merged
merged 5 commits into from
Jan 8, 2021
Merged

Conversation

sungly
Copy link
Owner

@sungly sungly commented Jan 6, 2021

Description:

  • added SRP authentication
  • updated Winston and prompt versioning to fix node incompatibility issues
  • updated readme.

How to test locally

npm run cognito srp-login and login with your username and password

@sungly sungly force-pushed the feature/add-srp-login branch from ec3f21b to a4aced1 Compare January 6, 2021 20:48
Copy link
Collaborator

@rocellaj rocellaj left a comment

Choose a reason for hiding this comment

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

just a quick review, i can test later with you as well. Just minor comments.

src/cmds/authentication/srp-login.js Outdated Show resolved Hide resolved
src/cmds/authentication/user-password-login.js Outdated Show resolved Hide resolved
@@ -28,9 +29,14 @@ const cli = () => {

break;
case 'login':
logger.info('@NOTE: Only user password auth is supported ATM.');
Copy link
Collaborator

Choose a reason for hiding this comment

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

thoughts of doing another prompt here - or providing option of which type of login ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

hmmm I'm okay just leaving as is since we're only going to have two types.

Copy link
Owner Author

Choose a reason for hiding this comment

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

we could tackle this on a different ticket?

src/cmds/authentication/srp-login.js Outdated Show resolved Hide resolved
src/cmds/authentication/srp-login.js Show resolved Hide resolved
Copy link
Collaborator

@rocellaj rocellaj left a comment

Choose a reason for hiding this comment

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

Lgtm

@sungly sungly merged commit 1890ebf into master Jan 8, 2021
sungly added a commit that referenced this pull request Jan 8, 2021
* update prompt and Winston versions

* update logger import

* update read me

* moved addSecretHashToParams to hash util
sungly pushed a commit that referenced this pull request Jan 8, 2021
# [1.4.0](v1.3.0...v1.4.0) (2021-01-08)

### Features

* **auth:** add SRP authentication ([#25](#25)) ([9f5aedc](9f5aedc))
@sungly sungly deleted the feature/add-srp-login branch January 9, 2021 12:55
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.

2 participants