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

Logout not working in Firefox latest version (128.0) #521

Closed
omasseau opened this issue Jul 16, 2024 · 9 comments · Fixed by #525
Closed

Logout not working in Firefox latest version (128.0) #521

omasseau opened this issue Jul 16, 2024 · 9 comments · Fixed by #525

Comments

@omasseau
Copy link

omasseau commented Jul 16, 2024

Environment
  • lua-resty-openidc 1.7.6
  • OpenID Connect provider : Keycloak 22
Expected behaviour

Logout works in Firefox

Actual behaviour

Logout is not working anymore in Firefox (version 128.0)
It just returns a 200 response instead of a 302. So no redirect happens.

Strangly an image(?!) is returned :
image

Minimized example

Just doing :

    location /logout {
        access_by_lua_block {
            opts= {
                ...
                logout_path = "/logout",
                ...
            }
            require("resty.openidc").authenticate(opts)
        }
    }

Configuration and NGINX server log files

access.log :
127.0.0.1 - - [16/Jul/2024:15:18:24 +0200] [34bff32f803d305d47feb962e13222c3] "GET /logout HTTP/1.1" 200 79 "http://fr.localtest.com:92/" "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:128.0) Gecko/20100101 Firefox/128.0" 0.000 s (-/-)

error.log :

2024/07/16 15:18:24 [debug] 12696#13360: *253 [lua] openidc.lua:1484: authenticate(): Logout path (/logout) is currently navigated -> Processing local session removal before redirecting to next step of logout process
2024/07/16 15:18:24 [debug] 12696#13360: *253 [lua] openidc.lua:560: openidc_discover(): openidc_discover: URL is: http://127.0.0.1:8180/auth/realms/Business-Document/.well-known/openid-configuration
2024/07/16 15:18:24 [debug] 12696#13360: *253 [lua] openidc.lua:678: openidc_get_token_auth_method(): 1 => private_key_jwt
2024/07/16 15:18:24 [debug] 12696#13360: *253 [lua] openidc.lua:72: supported(): Can't use private_key_jwt without opts.client_rsa_private_key
2024/07/16 15:18:24 [debug] 12696#13360: *253 [lua] openidc.lua:678: openidc_get_token_auth_method(): 2 => client_secret_basic
2024/07/16 15:18:24 [debug] 12696#13360: *253 [lua] openidc.lua:681: openidc_get_token_auth_method(): no configuration setting for option so select the first supported method specified by the OP: client_secret_basic
2024/07/16 15:18:24 [debug] 12696#13360: *253 [lua] openidc.lua:695: openidc_get_token_auth_method(): token_endpoint_auth_method result set to client_secret_basic
@omasseau omasseau changed the title Logout not working in Firefox since last update of Firefox Logout not working in Firefox latest version (128.0) Jul 16, 2024
@ross211
Copy link

ross211 commented Jul 17, 2024

I've just been looking at this too. It appears Firefox has added image/png and image/svg+xml to the default Accept header:

Firefox v115esr: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,                        */*;q=0.8
Firefox v128:    text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/png,image/svg+xml,*/*;q=0.8

This combined with the header check in openidc.lua is preventing it from doing the usual redirect and returning a blank image instead.

A workaround would be to override the header for requests to your logout path, eg:

    location /logout {
        rewrite_by_lua_block {
            ngx.req.set_header("Accept", "text/html")
        }
        access_by_lua_block {
            opts= {
                ...
                logout_path = "/logout",
                ...
            }
            require("resty.openidc").authenticate(opts)
        }
    }

more_clear_input_headers Accept; will also work if you're running openresty or an nginx that has the ngx_headers_more module loaded.

@omasseau
Copy link
Author

@ross211
Thanks, but won't there be any any drawbacks ?
I guess this header check in openidc.lua is there for a good reason. Maybe some logout scenarios won't work if forcing to bypass this header check by overriding the header ?

@ross211
Copy link

ross211 commented Jul 18, 2024

I think that header check is for a different method of triggering logout, probably so you can trigger a logout by calling the logout path from <img> tags of another page.

IMO if that behaviour is desired it should be triggered by passing a setting in the options instead of trying to guess based on the header content.

@omasseau
Copy link
Author

I think that header check is for a different method of triggering logout, probably so you can trigger a logout by calling the logout path from <img> tags of another page.

IMO if that behaviour is desired it should be triggered by passing a setting in the options instead of trying to guess based on the header content.

I agree, it would be nice to have an options to trigger or not this behaviour ;)

@vavra5
Copy link

vavra5 commented Jul 23, 2024

I have run into this issue as well. Seems like this was included since FF v128.0 and was to solve a 3+ yr old bug afaik.

https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/128#http
https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation/List_of_default_Accept_values

I have tested the fix proposed by @ross211 and this does indeed resolve the problem. I guess this could be a workaround because I think at least in my case the returned content-type for logout requests will always be "text/html". I don't know about other use cases though.

I would be interested to have one of the maintainers weigh in on the original purpose for this handling of "image/png" in the Accept headers. It looks like this has been in place since v1.1.

@makrste
Copy link

makrste commented Jul 26, 2024

Hi,

We are using a kong-oidc plugin that is based on lua-resty-openidc and ran into this issue with the latest firefox update too. As this was not expected, we get a black page with the logout url that just doesn't show anything (renders an "openidc_transparent_pixel" - how weird is this...?)

I understand the reasoning behind it, but i think this should rather be a configuration that an implicit assumption that if the client (browser or whatever) set the config, the navigation should be affected directly..

@francescodedomenico
Copy link

francescodedomenico commented Aug 12, 2024

hi,
I came accross this issue while using apache apisix, I have solved it by commenting the whole block checking the header and returning the png if it contains "image/png" in Accepts.

Even tho I do not understand this "GET Style logout" 998c7a6 cookies are wiped out only at client-side, an external authorization server will preserve it's sso cookies and you won't ever get logged out, that's why you typically need to perform a complete redirect cycle through every interested actor during a logout.

As far as I know OIDC is strongly based on redirects for front-channel and back-channel logout is handled with a POST http method.

OIDC standard wise, what is this for?

Thank you for your work

Update

After some reserach I did come up to where this png comes from, by following the comment on this on mod_auth_openidc

/* see if this is PF-PA style logout in which case we return a transparent pixel */
		const char *accept = oidc_http_hdr_in_accept_get(r);
		if ((_oidc_strcmp(url, OIDC_IMG_STYLE_LOGOUT_PARAM_VALUE) == 0) ||
		    ((accept) && _oidc_strstr(accept, OIDC_HTTP_CONTENT_TYPE_IMAGE_PNG))) {
			return oidc_util_http_send(r, (const char *)&oidc_logout_transparent_pixel,
						   sizeof(oidc_logout_transparent_pixel),
						   OIDC_HTTP_CONTENT_TYPE_IMAGE_PNG, OK);
		}

PF stands for PingFederate and it's using a custom, non-standard way to implement single logout for PingFederate SSO products, confirmation came from this comment:
https://support.pingidentity.com/s/question/0D5Do00000qtbsNKAQ/pf-support-for-oidc-session-and-logout-standards

Just my two cents on this, if a vendor proposes his own customization of the standard it shouldn't impact any OIDC RFC implementation.

@bodewig
Copy link
Collaborator

bodewig commented Aug 25, 2024

The code predates my involvement with the project. I believe what Ping Federate did back then was something that could have become a standard - but did not. This must have all happened before the front-channel logout spec has been finalized (it may have been an alternative design).

I discussed the issue with some coworkers and short of splitting the paths or removing the functionality completely the best way forward seemed to be proper Accept header parsing. If the User Agent would also accept text/html and would even prefer this, then don't return the pixel. This should also work for the newer Firefox versions.

@bodewig
Copy link
Collaborator

bodewig commented Aug 25, 2024

it would be great if anybody of you could give #525 a try

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 a pull request may close this issue.

6 participants