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

app crashing when no email returned in accesstoken by IDP (AzureAD) #507

Open
xmlking opened this issue Apr 24, 2024 · 4 comments
Open

app crashing when no email returned in accesstoken by IDP (AzureAD) #507

xmlking opened this issue Apr 24, 2024 · 4 comments

Comments

@xmlking
Copy link
Contributor

xmlking commented Apr 24, 2024

I am trying hasura-auth with AzureAD, building webapp with hasura-auth-js SDK.
my app crashing after trying to login with AzureAD SSO. here is the logs

error in hasura-auth logs

auth          | {"time":"2024-04-24T02:53:43.499422588Z","level":"WARN","msg":"email didn't pass access control checks","trace":{"trace_id":"01d71ae9-7306-4a72-884e-78b4f6fbafde","span_id":"","parent_span_id":""},"request":{"client_ip":"192.168.65.1","method":"POST","url":"/v1/token"}}
auth          | {"time":"2024-04-24T02:53:43.501355046Z","level":"ERROR","msg":"call completed with errors","trace":{"trace_id":"01d71ae9-7306-4a72-884e-78b4f6fbafde","span_id":"","parent_span_id":""},"request":{"client_ip":"192.168.65.1","method":"POST","url":"/v1/token"},"response":{"status_code":200,"latency_time":2491666,"errors":["json: error calling MarshalJSON for type types.Email: email: failed to pass regex validation"]}}

webapp crash logs
image
image

accesstoken return for AzureAD don't have email at right place
image

BUT this should not cause server crash...

Tasks

No tasks being tracked yet.
@xmlking
Copy link
Contributor Author

xmlking commented May 14, 2024

Just a note, apparently Microsoft have different AD subscriptions, based on plan, expensive corporate plan adds email claim correctly but for low cost plan, email claim is added at upn and unique_name fields. (we have to upgrade to expensive plan to customize claims)

Since getting email claim consistently is crucial for hasura-auth/hasura-auth-js to work, we need custom configuration option for AzureAD provider to map email field OR automatically fallback to upn or unique_name if email field is null or undefined.
Please let me know if I can contribute backward compatible fix via PR

@dbarrosop
Copy link
Member

Please let me know if I can contribute backward compatible fix via PR

Of course, it will be more than appreciated/welcomed :)

@dbarrosop
Copy link
Member

@xmlking would you mind checking if the issue still persists in 0.32.0? While AzuredAD's email is probably not being populated yet we remove the requirement of having an email so the crash should be gone.

@xmlking
Copy link
Contributor Author

xmlking commented Jul 4, 2024

@xmlking would you mind checking if the issue still persists in 0.32.0? While AzuredAD's email is probably not being populated yet we remove the requirement of having an email so the crash should be gone.

Yes crash issue is fixed with recent changes
But I will be contributing fallback email mapping for azure AD when I find little time
Thanks

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

No branches or pull requests

2 participants