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

Azure AD: login is unsuccessful if 'Username:' field is not populated #495

Open
arun-898 opened this issue Aug 16, 2022 · 11 comments
Open
Labels

Comments

@arun-898
Copy link

arun-898 commented Aug 16, 2022

I am trying to set up vouch proxy with Azure AD B2C with Drupal app using the following example

I have tried the following two providers.
Azure provider:
This fails immediately at /auth request because of the missing access token, so I decided to use ADFS
Error: /auth Error while retrieving user info after successful login at the OAuth provider: oauth2: server response missing access_token

ADFS Provider:
it results in too many redirects, because of failed authentication it keeps on redirecting to vouch proxy login URL. (all request redirects to 302) All the logs for ADFS are attached here
Vouch and nginx config added here

Can anyone please help me to resolve this issue?


Above logs and config are added for the previous (v0.27.1 ) of VP.
Added new logs for VP v0.37.3 Config
Logs

@bnfinet
Copy link
Member

bnfinet commented Aug 16, 2022

@arun-898 thanks for the detailed report.

It may be related to #445, that appears to be the same behavior.

I'm not an ADFS or Azure user and I'm not in a good position to assess this bug.

Are you able to populate the Username field as per the following? I'm wondering if that would change the behavior.

2022-08-16T13:14:52.481Z DEBUG User Obj: &{Username: Name: Email:[email protected] CreatedOn:0 LastUpdate:0 ID:0 TeamMemberships:[]}

In general I'd like to migrate VP to use to sub for it's primary user identifier but that needs some thought and work, which is partly why #445 and other related PRs have sat there for so long.

@arun-898
Copy link
Author

Thanks @bnfinet for the quick response, I am trying to populate Username with available attributes.

@arun-898
Copy link
Author

arun-898 commented Aug 17, 2022

I was not able to populate the username but I noticed some issues with claims.
After removing claims from the vouch config and Nginx config I was able to navigate to my app URL with testing: true in vouch config, but when I keep testing: false it results in too many redirects

Update:
Please ignore this, I misunderstood this with testing: false I was able to redirect to the app URL but when I click on the app URL, it was again redirecting me to the IDP. This is fixed now with the latest config.

@bnfinet
Copy link
Member

bnfinet commented Aug 17, 2022

@arun-898 I notice you're using
v0.27.1 of Vouch Proxy. Could you please update your version of VP

If you still have problems after upgrade, please do offer logs and config.

@arun-898
Copy link
Author

Thanks @bnfinet for the info, I have updated the latest VP tag v0.37.3, I will share the latest logs.

@arun-898
Copy link
Author

@bnfinet In the latest logs also I am getting the same error.
2022-08-17T19:00:06.452Z WARN no User found in jwt
is it possible to override other files in the vouch proxy repository, I am able to override config.yml only
image: quay.io/vouch/vouch-proxy:alpine-latest volumes: - ./vouch/config/config.yml:/config/config.yml

Can you please suggest how I can override the required files, I can then debug further for populating the username.
id_token returned from Azure does not have UPN field, so most likely I would have to alter
vouch/pkg/structs/structs.go // getname from Name field instead of UPN field.
func (u *ADFSUser) PrepareUserData() { u.Username = u.Name }

@bnfinet
Copy link
Member

bnfinet commented Aug 17, 2022

@arun-898 could you please publish those logs and the current config you're using to a gist as before?

Can you please suggest how I can override the required files

The best way to do that is to download the vouch-proxy source code from this git repo and then after modifying the repo run ./do.sh dbuildalpine which will build a new alpine docker image which you can use.

@arun-898
Copy link
Author

@bnfinet I have updated the logs and config for Vouch Proxy v0.37.3 in the ticket description, please have a look at the logs.

@arun-898
Copy link
Author

@bnfinet I am able to login using vouch proxy after I make the following changes to the custom docker image, username is populated in vouch logs.
vouch/pkg/structs/structs.go // getname from Name field instead of UPN field.
func (u *ADFSUser) PrepareUserData() { u.Username = u.Name }

To build the custom docker image ./do.sh dbuildalpine, I had to do two changes

  1. Change the golang version here vouch/Dockerfile.alpine from 1.16 to 1.19 in line 3 FROM golang:1.19 AS builder
  2. In do.sh file I had to set the directory manually SCRIPT="vouch" // current directory.

@bnfinet
Copy link
Member

bnfinet commented Aug 18, 2022

@arun-898 I'm glad it's working for you with those changes. I think that confirms what we already know from #445.

I'd be very curious to understand why it was working with testing: true but not with testing: false.

@arun-898
Copy link
Author

arun-898 commented Aug 19, 2022

@bnfinet yes Changes added here #445 will be required for ADFS user object as well until we move to sub logic #310

I'd be very curious to understand why it was working with testing: true but not with testing: false.

Please ignore this, I misunderstood this with testing: false I was able to redirect to app URL but when I click on the app URL, it was again redirecting me to the IDP. This is fixed now with latest config.

@bnfinet bnfinet added bug and removed question labels Aug 29, 2022
@bnfinet bnfinet changed the title Azure AD : Too many redirects because of unsuccessful authentication. Azure AD: login is unsuccessful if 'Username:' field is not populated Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants