Feature Show Logged in account email or client ID for azd auth login and azd auth login --check-status

[john0isaac]

closes #1745

• Exported auth.Manager.GetSignedInAccount()
• Got the email and displayed it in the message.
• Handel err in getting the account by printing the logged in message and logging the error.
• Handel service principal empty account.
• Implement a function to get client id for service principal
• Got the client id for service principal and displayed it.
• Handel empty client id.

Full flow of azd auth login and azd auth login --check-status with normal account

Full flow of azd auth login and azd auth login --check-status with service principal

[john0isaac]

@ellismg I learned about service principal and handled their exception.

[ellismg]

Still a little confused on how this ends up working. From your screenshot, it looks like you did the right thing, but I'm confused on how it works with the current diff.

My belief is that when you are logged in with a service principal, the call to readUserProperties in auth/manager is going to return a user properties struct that has a nil HomeAccountID and so GetSignedInAccount will return (nil, nil). I think that will cause a panic when we try to dereference the PerferredUsername.

Were there additional commits that fixed this up that you just missed pushing? Or is my mental model on how this code works incorrect?

[john0isaac]

(L343) This is the line that gets triggered to print the service principal client ID note that in the screen shot that I shared before I was trying to log in not trying to check-status that's why it's not consistent in the code.

Let me think about it and get back to you, I found that there is a difference between logging in and checking the status so, I will try to make the whole code more consistent as it's as confusing to me as it is to you.

Update 1:

In this commit dc1b59b I added the feature to display the logged in user email or service principal on successful login to make the code consistent

output of command azd auth login for normal account

output of command azd auth login for service principal

Currently trying to figure out how to get the service principal client id from something other than the flags to display it on the ---check-status output

Update 2:

I couldn't get the client id when a service principal account loggs in so, I implemented a new function in the auth.Manager to get it. f3f830a
Suggestion:
In the auth.Manager.GetSignedInAccount(ctx) we can return the currentUser.ClientID instead of returning nil but I didn't know the implications of doing this so, I opted for the safe path which is to leave it as it us and create a new function. (Can't we return it in the GetSignedInAccount?)

Full flow of azd auth login and azd auth login --check-status with normal account

Full flow of azd auth login and azd auth login --check-status with service principal

[vhvb1989]

azd supports logging in with:

• Using client-id
  • service principal (secret pass)
  • service principal (certificate)
  • Federated Token
• External auth
  • using az cli as token provider
• Using web browser
  • device-code
  • Interactive auth

I wouldn't like the idea of adding a new function for each supported scenario like GetLoggedIn<scenario-name>(ctx)

My recommendation is:

• Keep getSignedInAccount() private
• Create a new structure in auth package (auth/manager.go) like type LogInDetails struct
• Define a loginType string and account string inside LogInDetails (to start with)
• Adda new method for the Manager in auth package like LogInDetails(ctx) (*LogInDetails, error) {} and implement it for returning the information for the actual log in.
  • Create string enum for the login type.
• Use the LogInDetails results from the auth login command.

This approach would allow future iterations to easily add more information about the login (for example, adding the date of logged in, if device-code was used, etc.). We would also let the auth-Manager to be the one who knows how to recover/pull the login info details, w/o the azd-command trying to pull each type to guess the one that was used.

@ellismg for reviewing the proposal.

[john0isaac]

I wouldn't like the idea of adding a new function for each supported scenario like GetLoggedIn(ctx)

not sure if this is the case that we have here.

The current function getSignedInAccount() works with any type of login the problem that made me create another one is that getSignedInAccount() returns nil if you are signing in with a service principal account so If you are signing in with a normal email whatever sign-in method you are using getSignedInAccount() will work as it has been working.

That's why I created the other function to get the service principal without changing the current functionality of getSignedInAccount() not because it's a different scenario of login.

I'm okay with implementing your idea so, let's wait for Matt's review.

[vhvb1989]

My proposal's main intention is having a design which allows adding more information about the login in the future, like the type, time, etc.

It is not 100% required to complete the requirements from the associated issue, in this case. But I would take the change to set the foundation that enables getting all details about the login.

[john0isaac]

I understand, let's wait for Matt and see.

[john0isaac]

@rajeshkamal5050 we need Matt's input on it, what approach should we take in resolving this?

@vhvb1989 suggested a plan for it and I'm happy to implement but was waiting for any feedback from Matt to avoid rework.

[rajeshkamal5050]

@ellismg @vhvb1989 are we still looking into this enhancement and is the PR still valid?

@john0isaac can we close this and open a new one incorporating feedback?