-
Notifications
You must be signed in to change notification settings - Fork 53
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
Set up ng-dev auth and service #720
Conversation
Establish a base set of ng-dev token functions allowing for the request, validation and revocation of newly defined ng-dev tokens. These tokens will be used to allow ng-dev users to authenticate and request data from or actions to be performed by the dev-infra firebase instance.
Create a set of auth commands to login to the ng-dev service. Currently only supports logging in, logging out and restoring a logged in session.
ng-dev/auth/shared/ng-dev-token.ts
Outdated
/** Save the token to the file system as a base64 encoded string. */ | ||
async function saveTokenToFileSystem(data: NgDevToken) { | ||
await mkdir(tokenDir, {recursive: true}); | ||
await writeFile(tokenPath, Buffer.from(JSON.stringify(data), 'utf8').toString('base64')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you considered using some actual encrypting here where we store the secret in the bundled ng-dev
tool. This is obviously not safe, but it's much more difficult to figure out if a token is stolen, compared to just decoding base64 (where it's also super easy to see if something is base64 or not :D)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had not thought about it too much, but we definitely can if you would like. We would have to be shipping to the key for the encryption as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think this is worth doing. It's low-effort and makes potentially leaked keys much more difficult to become useful for attackers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice!! some minor final comments
a9815b3
to
1993089
Compare
1993089
to
b57325e
Compare
b57325e
to
469f0c6
Compare
This PR was merged into the repository by commit 114c5a9. |
Create a set of auth commands to login to the ng-dev service. Currently only supports logging in, logging out and restoring a logged in session. PR Close #720
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
No description provided.