Skip to content

Store credentials in a config directory #3092

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

Merged
merged 10 commits into from
Sep 14, 2021

Conversation

sigurdm
Copy link
Contributor

@sigurdm sigurdm commented Sep 2, 2021

Fixes: #2999

@sigurdm sigurdm requested a review from jonasfj September 2, 2021 10:39
@google-cla google-cla bot added the cla: yes label Sep 2, 2021
final legacyCredentialsFile = _legacyCredentialsFile(cache);
if (entryExists(legacyCredentialsFile)) {
log.message('Also deleting legacy credentials at $legacyCredentialsFile');
deleteEntry(legacyCredentialsFile);
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it make more sense if we move legacy credentials file into new path instead of deleting? It would be backwards-compatible with CI jobs that uses credentials.json for authentication & publishing packages.

Copy link
Contributor Author

@sigurdm sigurdm Sep 3, 2021

Choose a reason for hiding this comment

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

Here we are in the dart pub logout code - it should delete any credentials, so I don't think it makes sense to keep the legacy ones.

In the code for _credentialsFile below we use the credentials file from the legacy location if it exists there. We could consider moving it, but that would make it hard(er) to switch between older and newer versions of pub.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh sorry, 😅 haven't noticed it's for logout command.

@@ -89,7 +89,7 @@ class SystemCache {
/// Defaults to `false`.
SystemCache({String rootDir, bool isOffline = false})
: rootDir = rootDir ?? SystemCache.defaultDir,
tokenStore = TokenStore(rootDir ?? SystemCache.defaultDir) {
tokenStore = TokenStore(dartConfigDir) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's also rename tokens.json to pub-tokens.json.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes!
Done!

@sigurdm sigurdm merged commit 98a01e1 into dart-lang:master Sep 14, 2021
@sigurdm sigurdm deleted the secrets_not_in_cache branch September 14, 2021 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not store credentials in PUB_CACHE
3 participants