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.

Sign up for GitHub

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

Fix for DUO Universal Prompt authentication #426

Merged
merged 1 commit into from
Jan 30, 2024
Merged

Fix for DUO Universal Prompt authentication #426

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion aws_adfs/_duo_universal_prompt_authenticator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def _perform_authentication_transaction(
ssl_verification_enabled,
):
duo_host = re.sub(
r"/frame/frameless/v4/auth.*",
r"/frame/frameless/v\d+/auth.*",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised it isn't needed to capture this version returned the Duo server, and reuse it in the constructed URLs sent to the same server afterward:

# rg -F 'v4' aws_adfs/_duo_universal_prompt_authenticator.py 
219:    status_for_url = duo_host + "/frame/v4/status"
246:    result_for_url = duo_host + "/frame/v4/oidc/exit"
267:    status_for_url = duo_host + "/frame/v4/status"
537:    duo_url = duo_host + "/frame/v4/prompt"
574:    prompt_for_url = duo_host + "/frame/v4/prompt"

Does everything really work fine with this single change?

Copy link
Contributor Author

@SMxJrz SMxJrz Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @pdecat,

We've been using the tool against the DUO traditional prompt in our environment for a while with no issue and when our org made the switch over to the new Universal prompt, I had noticed that the latest version of the library started returning 403 when I tried to get credentials using the tool.

What I observed was that the duo_url before the regex replace call where my change is was making call out to "https://api-[API_ID].duosecurity.com/frame/frameless/v3/auth?sid=[[SID]]&tx=[[TX_ID]]/frame/v4/prompt"

The problem is, the goal of the regex wants to strip out the "frame/frameless/v3/auth?sid=[SID]&tx=[TX_ID]" part but the existing code is trying to match "/frame/frameless/v4/auth.*" and strip it out by replacing it with empty string, so you just have the URL to the DUO API.

The observation that I made, however, was that our DUO API was returning a different versioned /frame/frameless/v3/auth.* which led to the duo_url value in the having the frame/frameless/v3/auth?sid=[[SID]]&tx=[[TX_ID]] in the variable. which led to the following output

Error: Issues during beginning of the authentication process. The error response <Response [403]>

Which is due to the fact that the first call after this code (to /frame/v4/prompt) was not actually being called since the code was invoking "/frame/frameless/v3/auth?sid=[[SID]]&tx=[[TX_ID]]/frame/v4/prompt", where the "/frame/v4/prompt" part was part of tx in the query string of the URI.

Once I tweaked the regex to look for any digit the URI was stripped as expected from the DUO Url and we were able to get our credentials.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense, thanks for the detailed explanation.

"",
duo_url,
)
Expand Down