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

fix login if saml response is in body comment #59

Merged
merged 2 commits into from
May 4, 2023

Conversation

ByteCommander
Copy link
Contributor

@ByteCommander ByteCommander commented Sep 30, 2022

fixes #51

If there are no SAML and/or prelogin-cookie related headers found in the server responses, this PR adds the functionality to check the response body for those values in embedded XML documents inside any HTML body comments.

gp_saml_gui.py Outdated Show resolved Hide resolved
gp_saml_gui.py Show resolved Hide resolved
gp_saml_gui.py Outdated Show resolved Hide resolved
@rezekdan
Copy link

rezekdan commented Oct 7, 2022

tested this within our company and it works now

@ByteCommander
Copy link
Contributor Author

Any further updates or review comments on this, before it could get merged? @dlenski

@fang0654
Copy link

fang0654 commented Dec 6, 2022

Just chiming in that one of the VPNs I've been using stopped working overnight, and it turns out this was the issue, and this PR fixed it!

@messiahUA
Copy link

I hope this will be merged eventually.

gp_saml_gui.py Outdated Show resolved Hide resolved
@dlenski
Copy link
Owner

dlenski commented May 4, 2023

Thank you @ByteCommander for your patience and refinement here.

I tweaked the PR slightly (squash commits, cleanup), and tested it against OpenConnect's fake-gp-server.py (which is the only GP SAML server tha I have access to right now 🤷‍♂️)

  1. In OpenConnect v9.10 directory:
$ cd openconnect/tests
$ ./fake-gp-server.py localhost 80000 certs/server-{cert,key}.pem &
  1. Configure the fake server for SAML on the portal, with handoff to the gateway login, both using the portal-userauthcookie:
$ curl -k https://localhost:8000/CONFIGURE -d portal_saml=portal-userauthcookie -d portal_cookie=portal-userauthcookie
$ curl -k https://localhost:8000/CONFIGURE
Current configuration of fake GP server configuration:
TestConfiguration(gateways=('Default gateway',), portal_2fa=None, gw_2fa=None, portal_cookie='portal-userauthcookie', portal_saml='portal-userauthcookie', gateway_saml=None)
  1. Use gp-saml-gui localhost:8000 to do SAML authentication, verify that it works.

ByteCommander and others added 2 commits May 4, 2023 13:00
Fixes dlenski#51

[DL: Some GlobalProtect VPNs apparently return the crucial username and
cookie result fields *only* in HTML comments and *not* in HTTP headers.  In
order to handle these cases correctly, we must parse the HTML comments in
addition to the headers.]

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
This is a workaround for timing/race conditions.
@dlenski dlenski dismissed their stale review May 4, 2023 20:01

updated it myself

@dlenski dlenski merged commit ee5314a into dlenski:master May 4, 2023
6 checks passed
jollaitbot pushed a commit to sailfishos-mirror/openconnect that referenced this pull request Sep 22, 2023
…ly in comments

This modifies the fake GP server to have a 'saml_comments_only' option.  If
set, the SAML completion fields ('saml-username', 'prelogin-cookie', etc.)
will be sent to the client *only* in a blob of XML wrapped in HTML comments,
and *not* in HTTP headers.

Some real GP servers are known to behave like this, and authentication
handlers like 'gp-saml-gui' need to be able to handle this case correctly
(see dlenski/gp-saml-gui#51 and
dlenski/gp-saml-gui#59).

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
jollaitbot pushed a commit to sailfishos-mirror/openconnect that referenced this pull request Sep 23, 2023
…ly in comments

This modifies the fake GP server to have a 'saml_comments_only' option.  If
set, the SAML completion fields ('saml-username', 'prelogin-cookie', etc.)
will be sent to the client *only* in a blob of XML wrapped in HTML comments,
and *not* in HTTP headers.

Some real GP servers are known to behave like this, and authentication
handlers like 'gp-saml-gui' need to be able to handle this case correctly
(see dlenski/gp-saml-gui#51 and
dlenski/gp-saml-gui#59).

Signed-off-by: Daniel Lenski <dlenski@gmail.com>
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

Successfully merging this pull request may close these issues.

gp-saml-gui doesnt works on last palo alto updates (cookie is not in headers)
6 participants