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

feat: Cache interactive login credentials #222

Merged
merged 13 commits into from
Aug 8, 2019

Conversation

PIC123
Copy link
Contributor

@PIC123 PIC123 commented Aug 2, 2019

feat: Cache interactive login credentials

Currently once a user logs in using the interactive login, there is no way to cache the credentials and the user must keep logging in through that method. This adds a simple file token cache that saves the credentials and subscription information and then uses those to login, reducing number of logins required.

AB#359

Copy link
Contributor

@wbreza wbreza left a comment

Choose a reason for hiding this comment

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

Great progress - Let me know if you need help wrapping this up.

src/plugins/login/azureLoginPlugin.ts Outdated Show resolved Hide resolved
src/services/baseService.ts Outdated Show resolved Hide resolved
src/services/loginService.ts Outdated Show resolved Hide resolved
src/services/loginService.ts Outdated Show resolved Hide resolved
src/services/loginService.ts Outdated Show resolved Hide resolved
const CONFIG_DIRECTORY = path.join(os.homedir(), ".azure");
const SLS_TOKEN_FILE = path.join(CONFIG_DIRECTORY, "slsTokenCache.json");

export class SimpleFileTokenCache {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an actual interface type definition that we can reference here?

src/plugins/login/utils/simpleFileTokenCache.ts Outdated Show resolved Hide resolved
src/plugins/login/utils/simpleFileTokenCache.ts Outdated Show resolved Hide resolved
src/plugins/login/utils/simpleFileTokenCache.ts Outdated Show resolved Hide resolved
src/services/loginService.ts Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Aug 8, 2019

Coverage Status

Coverage decreased (-0.6%) to 83.74% when pulling 6dadb40 on phcherne/interactive-login-caching into b0c5ef7 on dev.

src/services/baseService.ts Outdated Show resolved Hide resolved
src/services/loginService.ts Outdated Show resolved Hide resolved
src/plugins/login/azureLoginPlugin.test.ts Show resolved Hide resolved
src/services/loginService.ts Outdated Show resolved Hide resolved
src/test/mockFactory.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@wbreza wbreza left a comment

Choose a reason for hiding this comment

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

Looks great!

Copy link
Contributor

@mydiemho mydiemho left a comment

Choose a reason for hiding this comment

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

looks good, have a few comments

this.load();
}

public add(entries: any, cb?: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's cb? a more descriptive name would be better

Copy link
Contributor

Choose a reason for hiding this comment

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

callback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a callback, I'll switch the name

}
}

public remove(entries: any, cb?: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Contributor

Choose a reason for hiding this comment

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

callback

}
}

public find(query: any, cb?: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same


//-------- File toke cache specific methods

public addSubs(entries: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

does entries change? can we be more specific than just default to any?


public addSubs(entries: any) {
this.subscriptions.push(...entries);
this.subscriptions = this.subscriptions.reduce((acc, current) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's acc stand for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's the accumulator for the reducer, not a very exciting name haha

@PIC123 PIC123 merged commit 1da74af into dev Aug 8, 2019
@wbreza wbreza deleted the phcherne/interactive-login-caching branch September 12, 2019 20:35
tbarlow12 pushed a commit that referenced this pull request Sep 13, 2019
* Add simple file token cache

* Address pr feedback

* Added tests for existing and nonexisting cached login

* Cleaned up cached login test for login service

* Clean up simpleFileTokenCache, add tests

* Add param to simpleFileTokenCache constructor for path

* Updated simple file token cache tests

* Restore fs mocks at end of each test

* Fix getTokens in mock

* Fixing broken tests

* Address pr feedback

* Pushing again

* Address more pr feedback
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.

5 participants