Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Support NEW VSTS Identity Service Requirement #57

Merged
merged 1 commit into from
Sep 27, 2016

Conversation

yacaovsnc
Copy link
Member

Based on commit 70af3d from Git-Credential_manager-for-Windows

Manual test:

  1. Compile and install gcm4ml locally
  2. Enable debug from .gitconfig
  3. Remove cached credentials (on Mac, it's saved in keychain.)
  4. Clone a repo hosted by VSTS and observe:
VsoAzureAuthority::populateTokenTargetId
VsoAzureAuthority::createConnectionDataRequest
   validating token
   target identity is add8a40a-597c-43ff-93fa-e0153bc896ab
   parsed identity service url: https://app.vssps.visualstudio.com/Aadd8a40a-597c-43ff-93fa-e0153bc896ab/
   creating access token scoped to 'vso.code_write' for 'add8a40a-597c-43ff-93fa-e0153bc896ab'
   personal access token acquisition succeeded.
SecretStore::writeCredentials

Verify clone is successful and credential is saved properly.

Copy link

@AlexRukhlin AlexRukhlin 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 for me

if (identityServiceUri != null)
{
Trace.writeLine(" parsed identity service url: " + identityServiceUri);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This naming confused me a little, that parseLocationServiceUriFromJson() is used to retrieve the identityServiceUri. Can you add a code comment to explain it?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, it's a bad name. I am not parsing location "service", I am only parsing the "location" field, I renamed the method to reflect that.

@davidstaheli
Copy link
Contributor

Looks good. Added one comment.

Based on commit 70af3d from Git-Credential_manager-for-Windows
microsoft/Git-Credential-Manager-for-Windows@70af3d7
@yacaovsnc yacaovsnc merged commit e633ec8 into microsoft:master Sep 27, 2016
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants