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

[Feature Request]: Adding logoff POST call #1575

Closed
danielebonfatti opened this issue Nov 8, 2022 · 8 comments
Closed

[Feature Request]: Adding logoff POST call #1575

danielebonfatti opened this issue Nov 8, 2022 · 8 comments
Assignees

Comments

@danielebonfatti
Copy link

danielebonfatti commented Nov 8, 2022

Hi all,
since the OpenID specs https://openid.net/specs/openid-connect-rpinitiated-1_0.html say that

OpenID Providers MUST support the use of the HTTP GET and POST methods defined in RFC 7231 [RFC7231] at the Logout Endpoint

and some OpenID Providers expose also the POST endpoint, would be possible to add the possibility to call logoff with POST method, in a configurable way?
At the moment the call is always a GET (logoff-revocation.service.ts):

// Logs out on the server and the local client.
// If the server state has changed, check session, then only a local logout.
logoff(config: OpenIdConfiguration, allConfigs: OpenIdConfiguration[], authOptions?: AuthOptions): void {
  const { urlHandler, customParams } = authOptions || {};

  this.loggerService.logDebug(config, 'logoff, remove auth ');

  const endSessionUrl = this.getEndSessionUrl(config, customParams);

  this.resetAuthDataService.resetAuthorizationData(config, allConfigs);

  if (!endSessionUrl) {
    this.loggerService.logDebug(config, 'only local login cleaned up, no end_session_endpoint');

    return;
  }

  if (this.checkSessionService.serverStateChanged(config)) {
    this.loggerService.logDebug(config, 'only local login cleaned up, server session has changed');
  } else if (urlHandler) {
    urlHandler(endSessionUrl);
  } else {
    this.redirectService.redirectTo(endSessionUrl);
  }
}

Our security office told us not to use the GET call for "security reasons". Basically it is a prudential position due to the fact that my company belongs to a bigger group. They claim that passing any kind of information in clear is somehow a potential threat to the security, even if the data passed is not critical.

Thank you in advance

Best regards

@damienbod
Copy link
Owner

damienbod commented Nov 8, 2022

This makes sense. We will added support for HTTPs POST logout

Per default => POST
Per config => use GET

?

@danielebonfatti
Copy link
Author

Sounds good, thank you.
Do you have any plan (even rough) about the realease of this feature?

@FabianGosebrink
Copy link
Collaborator

We have not even started implementing it :), But it does make sense. We will provide appropriate methods. I think I can start on the weekend at the latest. Thanks!

@FabianGosebrink FabianGosebrink self-assigned this Nov 8, 2022
@danielebonfatti
Copy link
Author

Thanks @FabianGosebrink

@fritzwf
Copy link

fritzwf commented Nov 9, 2022

The documents say that you can subscribe on logoff(), but it doesn't seem to be there anymore. Isn't the logoff() operation asynchronous since it interacts with the OIDC server?

@FabianGosebrink
Copy link
Collaborator

FabianGosebrink commented Nov 10, 2022

Where do the docs tell that you can subscribe to a logoff? Could not find it. Thanks. Yes it is asynchronous, but as you are getting redirected, we are doing this internally. Would you like to have got the observable passed out, so that you can subscribe to it?

@FabianGosebrink
Copy link
Collaborator

This makes sense. We will added support for HTTPs POST logout

Per default => POST
Per config => use GET

?

This would change the default behavior, as the default is GET now.

@damienbod
Copy link
Owner

damienbod commented Nov 12, 2022

Lets leave this default => GET

set a new config, can use a POST

good?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants