[AzureAD] fix broken AzureAD integration #795

christianmeyer · 2022-03-23T22:53:40Z

I fixed the AzureAD integration.
The refactored code should support native logins and ADFS federated logins.
I tried to incorporate all existing edge cases, but could not test all of them.

christianmeyer · 2022-03-23T23:17:05Z

this might fix among others, issues #689 , #662 , #645 , #624 , #573 , ...

christianmeyer · 2022-03-25T13:10:11Z

As it might be a bit hard to follow the thought process behind the refactoring, I've reiterated the changes and created a separate branch to illustrate the way of this refactoring. I didn't want to push hundreds of minor commits, so only pushed a proper merge. Feel free to have a look at the details, if that helps to understand the huge change:
https://github.com/christianmeyer/saml2aws/tree/fix_azuread_in_steps

theemethod · 2022-03-30T03:32:34Z

I tried your v2.34.0-patch.1. I no longer receive the error I was getting but now receive: invalid character 'h' looking for beginning of value

christianmeyer · 2022-03-30T14:29:57Z

I tried your v2.34.0-patch.1. I no longer receive the error I was getting but now receive: invalid character 'h' looking for beginning of value

@theemethod would you be able to give a bit more context, like an anonymized verbose run?

my run, with MFA and ADFS path currently looks like:

DEBU[0000] Running               command=list-roles
Using IdP Account default to access AzureAD https://account.activedirectory.windowsazure.com
DEBU[0000] Get credentials       helper=osxkeychain serverURL="https://account.activedirectory.windowsazure.com"
DEBU[0000] Get credentials       helper=osxkeychain user=***
To use saved password just hit enter.
? Username ***
? Password ***

DEBU[0004] building provider     command=list idpAccount="account {\n  AppID: ***\n  URL: https://account.activedirectory.windowsazure.com\n  Username: ***\n  Provider: AzureAD\n  MFA: Auto\n  SkipVerify: false\n  AmazonWebservicesURN: urn:amazon:webservices\n  SessionDuration: 3600\n  Profile: saml\n  RoleARN: \n  Region: \n}"
DEBU[0005] HTTP Req              URL="https://login.microsoftonline.com/common/GetCredentialType?mkt=en-US" http=client method=POST
DEBU[0005] HTTP Res              Status="200 OK" http=client
DEBU[0005] HTTP Req              URL="https://sts.***/adfs/ls/?client-request-id=***" http=client method=POST
DEBU[0006] HTTP Res              Status="200 OK" http=client
DEBU[0006] HTTP Req              URL="https://login.microsoftonline.com:443/login.srf" http=client method=POST
DEBU[0006] HTTP Res              Status="200 OK" http=client
DEBU[0006] HTTP Req              URL="https://login.microsoftonline.com:443/kmsi" http=client method=POST
DEBU[0006] HTTP Res              Status="200 OK" http=client
DEBU[0006] HTTP Req              URL="https://account.activedirectory.windowsazure.com/" http=client method=POST
DEBU[0008] HTTP Res              Status="200 OK" http=client
DEBU[0008] HTTP Req              URL="https://login.microsoftonline.com:443/common/DeviceAuthTls/reprocess" http=client method=POST
DEBU[0008] HTTP Res              Status="200 OK" http=client
DEBU[0008] HTTP Req              URL="https://login.microsoftonline.com/common/SAS/BeginAuth" http=client method=POST
DEBU[0009] HTTP Res              Status="200 OK" http=client
? Enter verification code ******
DEBU[0072] HTTP Req              URL="https://login.microsoftonline.com/common/SAS/EndAuth" http=client method=POST
DEBU[0072] HTTP Res              Status="200 OK" http=client
DEBU[0072] HTTP Req              URL="https://login.microsoftonline.com/common/SAS/ProcessAuth" http=client method=POST
DEBU[0073] HTTP Res              Status="200 OK" http=client
***

I got a similar run for a non ADFS flow and also without MFA .. and I remember a conditional run via VPN also went fine. The only issue we faced at our company was an error out of mixing tenant and app IDs.

theemethod · 2022-03-30T14:49:11Z

I got a similar run for a non ADFS flow and also without MFA .. and I remember a conditional run via VPN also went fine. The only issue we faced at our company was an error out of mixing tenant and app IDs.

@christianmeyer Sure. I'm using Windows 10. You can ignore the cache errors. I was trying to figure out how to get the raw output that is failing. Not sure if that is possible though.

C:\Users\<user>>saml2aws login --verbose
time="2022-03-30T07:40:26-07:00" level=debug msg=Running command=login
time="2022-03-30T07:40:26-07:00" level=debug msg="Check if creds exist." command=login
time="2022-03-30T07:40:26-07:00" level=debug msg=Expand name="C:\\Users\\<user>/.aws/credentials" pkg=awsconfig
time="2022-03-30T07:40:26-07:00" level=debug msg=resolveSymlink name="C:\\Users\\<user>\\.aws\\credentials" pkg=awsconfig
time="2022-03-30T07:40:26-07:00" level=debug msg=ensureConfigExists filename="C:\\Users\\<user>\\.aws\\credentials" pkg=awsconfig
Using IdP Account default to access AzureAD https://account.activedirectory.windowsazure.com
To use saved password just hit enter.
? Username (<user>@<domain>) <user>@<tenant>.onmicrosoft.com

? Username <user>@<tenant>.onmicrosoft.com
? Password *********

time="2022-03-30T07:40:41-07:00" level=debug msg="building provider" command=login idpAccount="account {\n  AppID: <AppId>\n  URL: https://account.activedirectory.windowsazure.com\n  Username: <user>@<tenant>.onmicrosoft.com\n  Provider: AzureAD\n  MFA: Auto\n  SkipVerify: false\n  AmazonWebservicesURN: urn:amazon:webservices\n  SessionDuration: 43200\n  Profile: saml\n  RoleARN: arn:aws:iam::<account>:role/Azure_SSO_Full_Access\n  Region: us-west-1\n}"
time="2022-03-30T07:40:41-07:00" level=debug msg="Could not extract a valid expiry time for the MFA tokenmissing Response element" Cache_file="C:\\Temp\\saml2aws.txt" IdpAccount=default pkg=samlcache
time="2022-03-30T07:40:41-07:00" level=debug msg="Cache is invalid" command=login
Authenticating as <user>@<tenant>.onmicrosoft.com ...
time="2022-03-30T07:40:42-07:00" level=debug msg="HTTP Req" URL="https://login.microsoftonline.com/common/GetCredentialType?mkt=en-US" http=client method=POST
time="2022-03-30T07:40:42-07:00" level=debug msg="HTTP Res" Status="200 OK" http=client
time="2022-03-30T07:40:42-07:00" level=debug msg="HTTP Req" URL="https://login.microsoftonline.com/common/login" http=client method=POST
time="2022-03-30T07:40:42-07:00" level=debug msg="HTTP Res" Status="200 OK" http=client
invalid character 'h' looking for beginning of value
login response unmarshal error
github.com/versent/saml2aws/v2/pkg/provider/aad.(*Client).processAuthentication
        github.com/versent/saml2aws/v2/pkg/provider/aad/aad.go:556
github.com/versent/saml2aws/v2/pkg/provider/aad.(*Client).Authenticate
        github.com/versent/saml2aws/v2/pkg/provider/aad/aad.go:322
github.com/versent/saml2aws/v2/cmd/saml2aws/commands.Login
        github.com/versent/saml2aws/v2/cmd/saml2aws/commands/login.go:105
main.main
        github.com/versent/saml2aws/cmd/saml2aws/main.go:188
runtime.main
        runtime/proc.go:255
runtime.goexit
        runtime/asm_amd64.s:1581
error authenticating
github.com/versent/saml2aws/v2/pkg/provider/aad.(*Client).Authenticate
        github.com/versent/saml2aws/v2/pkg/provider/aad/aad.go:324
github.com/versent/saml2aws/v2/cmd/saml2aws/commands.Login
        github.com/versent/saml2aws/v2/cmd/saml2aws/commands/login.go:105
main.main
        github.com/versent/saml2aws/cmd/saml2aws/main.go:188
runtime.main
        runtime/proc.go:255
runtime.goexit
        runtime/asm_amd64.s:1581
Error authenticating to IdP.
github.com/versent/saml2aws/v2/cmd/saml2aws/commands.Login
        github.com/versent/saml2aws/v2/cmd/saml2aws/commands/login.go:107
main.main
        github.com/versent/saml2aws/cmd/saml2aws/main.go:188
runtime.main
        runtime/proc.go:255
runtime.goexit
        runtime/asm_amd64.s:1581

christianmeyer · 2022-03-30T22:40:45Z

@christianmeyer Sure. I'm using Windows 10. You can ignore the cache errors. I was trying to figure out how to get the raw output that is failing. Not sure if that is possible though.

@theemethod I was able to reproduce the error output under some MFA circumstances and patched it (MS seems kinda creative with their interruptions in the flow). Can you check if my v2.34.0-patch.2 does fix your issue?
... and btw, the raw output, to some extend, can be enabled by setting DUMP_CONTENT=true as described in the README

theemethod · 2022-03-31T03:43:57Z

@christianmeyer Sure. I'm using Windows 10. You can ignore the cache errors. I was trying to figure out how to get the raw output that is failing. Not sure if that is possible though.

@theemethod I was able to reproduce the error output under some MFA circumstances and patched it (MS seems kinda creative with their interruptions in the flow). Can you check if my v2.34.0-patch.2 does fix your issue? ... and btw, the raw output, to some extend, can be enabled by setting DUMP_CONTENT=true as described in the README

Thanks for the update. Unfortunately, the error output seems to be the same. This is a new testing tenant so there are no CA policies and MFA is disabled. I was able to get the raw output. Would any of that be useful?

christianmeyer · 2022-03-31T09:36:43Z

Thanks for the update. Unfortunately, the error output seems to be the same. This is a new testing tenant so there are no CA policies and MFA is disabled. I was able to get the raw output. Would any of that be useful?

@theemethod I've tested it against a brand new AzureAD setup after your feedback and disabled all Security Defaults to get down to the bare minimum flow, and it still works.
Can you first post an updated --verbose run here? Posting the full DUMP_CONTENT=true here, would eventually expose sensible information, even though it would be interesting to know where exactly it steps out.
I mean, you could grab the code and add a few log.Printf("...") around the area where the verbose run exits - if that's an option for you.
Otherwise feel free to drop me an email (see git log) ... but please make sure to obscure any passwords especially in the POST payloads

theemethod · 2022-03-31T22:21:37Z

Thanks for the update. Unfortunately, the error output seems to be the same. This is a new testing tenant so there are no CA policies and MFA is disabled. I was able to get the raw output. Would any of that be useful?

@theemethod I've tested it against a brand new AzureAD setup after your feedback and disabled all Security Defaults to get down to the bare minimum flow, and it still works. Can you first post an updated --verbose run here? Posting the full DUMP_CONTENT=true here, would eventually expose sensible information, even though it would be interesting to know where exactly it steps out. I mean, you could grab the code and add a few log.Printf("...") around the area where the verbose run exits - if that's an option for you. Otherwise feel free to drop me an email (see git log) ... but please make sure to obscure any passwords especially in the POST payloads

@christianmeyer So I tested in our production tenant and everything works. We are currently federated with ADFS and I also have an account configured for our staged rollout to password hash sync. Both accounts worked just fine. One difference is that our staging tenant uses our @*.onmicrosoft.com userprincipalname. I can't easily change this, at least not right now. I can come back to the staging tenant issue in a few days once I finish this production update.

Thank you for your assistance.

…ient

christianmeyer · 2022-04-02T13:01:12Z

Given the exceptions introduced as part of this discussions, I've refactored the code again, to isolate them in the thought to be somehow linear authentication flow. The AzureAD authentication flow varies a lot, dependent on the security settings within AzureAD thus can have KMSI interrupts at felt random spots, same goes for (non) MFA flows.

deepend-dev · 2022-06-10T05:23:27Z

Does this also solves issue with idp url error if the tenant is in other directory?

blang · 2022-11-04T09:43:37Z

Is there any update on merging this? Any support needed?

…azuread

… fix_azuread

christianmeyer · 2023-03-17T12:11:55Z

I've resolved the merge conflicts against master, that should incorporate the changes out of bd8ae8b, 3f9221e, 7ba431a, bcd6a14 and c0ae40a

codecov-commenter · 2023-03-17T23:01:46Z

Codecov Report

Merging #795 (c377703) into master (75b4721) will increase coverage by 3.81%.
The diff coverage is 66.41%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #795      +/-   ##
==========================================
+ Coverage   27.88%   31.70%   +3.81%     
==========================================
  Files          52       52              
  Lines        7379     7463      +84     
==========================================
+ Hits         2058     2366     +308     
+ Misses       5069     4787     -282     
- Partials      252      310      +58

Flag	Coverage Δ
unittests	`31.70% <66.41%> (+3.81%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/provider/aad/aad.go	`59.11% <66.41%> (+59.11%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

mapkon · 2023-03-21T23:59:10Z

I will merge this tomorrow

realtimerick · 2023-03-27T12:42:13Z

Hi guys, when is the version bump (to v2.36.6) expected?

mapkon · 2023-03-27T13:14:13Z

Definitely this week. Bare with me as I clear out time to release (apologies for the delay).

…

On Mon, Mar 27, 2023 at 22:42, realtimerick ***@***.***> wrote: Hi guys, when is the version bump (to v2.36.6) expected? — Reply to this email directly, view it on GitHub <#795 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGQV5PE57LDOB7XA3CKOHTW6GDK7ANCNFSM5RPM7Y3Q> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

refactored AzureAD authentication for AWS IAM federation

2be5a3b

christianmeyer added 2 commits March 31, 2022 00:30

fix various edge cases for not (yet) setup MFA

a37cca7

fail on ADFS login errors

15ba2e5

cope for the felt random interrupts and thus made the flow more resil…

e22b864

…ient

christianmeyer added 2 commits April 12, 2022 18:11

tests for AzureAD provider

5f7bd61

Merge branch 'master' into fix_azuread

f167db2

Merge branch 'master' into fix_azuread

97f4136

christianmeyer added 2 commits March 17, 2023 12:56

Merge branch 'master' of github.com:christianmeyer/saml2aws into fix_…

742de35

…azuread

Merge branch 'fix_azuread' of github.com:christianmeyer/saml2aws into…

e113a4a

… fix_azuread

fix typo introduced by prior merge

2ee4371

Merge branch 'master' into fix_azuread

c377703

mapkon merged commit 5ec1508 into Versent:master Mar 22, 2023

kieran-lowe mentioned this pull request Jun 12, 2023

Login not possible in AzureAD when System-preferred multifactor authentication is activated #1054

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AzureAD] fix broken AzureAD integration #795

[AzureAD] fix broken AzureAD integration #795

christianmeyer commented Mar 23, 2022

christianmeyer commented Mar 23, 2022

christianmeyer commented Mar 25, 2022

theemethod commented Mar 30, 2022

christianmeyer commented Mar 30, 2022

theemethod commented Mar 30, 2022

christianmeyer commented Mar 30, 2022

theemethod commented Mar 31, 2022

christianmeyer commented Mar 31, 2022

theemethod commented Mar 31, 2022

christianmeyer commented Apr 2, 2022

deepend-dev commented Jun 10, 2022

blang commented Nov 4, 2022

christianmeyer commented Mar 17, 2023

codecov-commenter commented Mar 17, 2023 •

edited

Loading

mapkon commented Mar 21, 2023

realtimerick commented Mar 27, 2023

mapkon commented Mar 27, 2023 via email

[AzureAD] fix broken AzureAD integration #795

[AzureAD] fix broken AzureAD integration #795

Conversation

christianmeyer commented Mar 23, 2022

christianmeyer commented Mar 23, 2022

christianmeyer commented Mar 25, 2022

theemethod commented Mar 30, 2022

christianmeyer commented Mar 30, 2022

theemethod commented Mar 30, 2022

christianmeyer commented Mar 30, 2022

theemethod commented Mar 31, 2022

christianmeyer commented Mar 31, 2022

theemethod commented Mar 31, 2022

christianmeyer commented Apr 2, 2022

deepend-dev commented Jun 10, 2022

blang commented Nov 4, 2022

christianmeyer commented Mar 17, 2023

codecov-commenter commented Mar 17, 2023 • edited Loading

Codecov Report

mapkon commented Mar 21, 2023

realtimerick commented Mar 27, 2023

mapkon commented Mar 27, 2023 via email

codecov-commenter commented Mar 17, 2023 •

edited

Loading