result.getRedirect() after Logout returns undefined

[VILLAN3LL3]

As #567 is closed, I open a new thread with the same issue:

The logout redirect does not work in my project, too. result.getRedirect() returns undefined (result is NbAuthResult). Where can I configure the logout redirect? Using the redirectUri config in forms/logout does not work. Neither does the configuration of a logout redirect uri in the Azure AD App registration (I'm using OAuth2 with Azure AD).

Issue type

I'm submitting a ... (check one with "x")

• [x ] bug report
• feature request

Issue description

Current behavior:
redirect after logout: result.getRedirect() returns undefined

Expected behavior:
not sure where the redirect url should be defined

Steps to reproduce:

Related code:
see #567

Other information:

npm, node, OS, Browser
node: 12.6.0
npm: 6.11.3
OS: Windows 10
Browser: Chrome

Angular, Nebular
"dependencies": {
"@agm/core": "^1.0.0-beta.5",
"@angular/animations": "^8.0.0",
"@angular/cdk": "^8.0.0",
"@angular/common": "^8.0.0",
"@angular/compiler": "^8.0.0",
"@angular/core": "^8.0.0",
"@angular/forms": "^8.0.0",
"@angular/platform-browser": "^8.0.0",
"@angular/platform-browser-dynamic": "^8.0.0",
"@angular/router": "^8.0.0",
"@nebular/auth": "4.1.2",
"@nebular/eva-icons": "4.1.2",
"@nebular/security": "4.1.2",
"@nebular/theme": "4.1.2",
"bootstrap": "4.3.1",
"classlist.js": "1.1.20150312",
"core-js": "2.5.1",
"eva-icons": "^1.1.0",
"intl": "1.2.5",
"ionicons": "2.0.1",
"nebular-icons": "1.1.0",
"ng-circle-progress": "^1.4.1",
"node-sass": "^4.12.0",
"normalize.css": "6.0.0",
"pace-js": "1.0.2",
"roboto-fontface": "0.8.0",
"rxjs": "6.5.2",
"rxjs-compat": "6.3.0",
"socicon": "3.0.5",
"typeface-exo": "0.0.22",
"web-animations-js": "github:angular/web-animations-js#release_pr208",
"zone.js": "~0.9.1"
},
"devDependencies": {
"@angular-devkit/build-angular": "~0.800.2",
"@angular/cli": "^8.0.2",
"@angular/compiler-cli": "^8.0.0",
"@angular/language-service": "8.0.0",
"@compodoc/compodoc": "^1.1.10",
"@types/jasmine": "2.5.54",
"@types/jasminewd2": "2.0.3",
"@types/node": "6.0.90",
"codelyzer": "^5.0.1",
"conventional-changelog-cli": "1.3.4",
"husky": "0.13.3",
"jasmine-core": "2.6.4",
"jasmine-spec-reporter": "4.1.1",
"karma": "1.7.1",
"karma-chrome-launcher": "2.1.1",
"karma-cli": "1.0.1",
"karma-coverage-istanbul-reporter": "1.3.0",
"karma-jasmine": "1.1.0",
"karma-jasmine-html-reporter": "0.2.2",
"npm-run-all": "4.0.2",
"protractor": "5.1.2",
"rimraf": "2.6.1",
"stylelint": "7.13.0",
"ts-node": "3.2.2",
"tslint": "^5.7.0",
"tslint-language-service": "^0.9.9",
"typescript": "3.4.5"
}

[wspurgin]

For clarity the offending lines are in the OAuth2Strategy:

  logout(): Observable<NbAuthResult> {
    return observableOf(new NbAuthResult(true));
  }

Basically, it doesn't actually implement a logout. That's effectively just returning "success!" without actually making any request against the OAuth provider. Interestingly, prior to the OAuth2 with Google example calling logout with OAuth2 would produce an error (like the other non-supported features). I don't know why that was replaced with pseudo-success 🤔 I couldn't tell by looking at the PR for it

My guess is that because the strategy was implemented with Google OAuth in mind, Google considered it an anti-pattern to have Apps that they have no control over sign their users out of their Ecosystem (and I don't think they even expose a "logout" endpoint for you). Therefore, your app stays "logged in" as long as they are logged into the OAuth provider (e.g., Google).

So when using google, this assumption doesn't break the paradigm, but it does mislead us (since using the Logout component with the OAuth2 strategy doesn't raise any errors or cause any problems). However, I still think there's a use case for having OAuth2 (ish) logout implemented.

The OP is using Azure AD, so I assume the app in which they're using Nebular (:heart: it by the way) and want a logout is actually running OpenID Connect on Azure AD. In which case, their use case is probably like mine.

I'm using Keycloak IAM for a suite of apps that my org all own (i.e. our own product ecosystem). We use OpenID Connect as the protocol (which is OAuth2 compatible). OpenID Connect's protocol does have a logout endpoint exposed, it's just guarded so that only our valid apps can actually logout our users (I may be mis-remembering but I believe Google does it that way themselves). In our instance, we want a "sign out" in our app ecosystem to be like logging out from GMail. It's actually directing that logout to the "Original Provider" (Google) and logging you out of that account.

To get the OpenID Connect logout endpoint to work, I had to extend the the Nebular OAuth2 strategy to just send the request to the logout endpoint:

// Extension of NbOAuth2AuthStrategyOptions is also required in order to create 
// the 'logout' action option. However once you have that in place, you would
// configure it like the other actions
// ...
// logout: {
//     endpoint: 'logout',
//     redirectUri: 'https://account.yourcompany.org/auth/openid/callback/logout'
// }

  protected buildLogoutUrl() {
      const params = {
        redirect_uri: this.getOption('logout.redirectUri'),
        ...this.getOption('logout.params'),
      };

    const endpoint = this.getActionEndpoint('logout');
    const query = this.urlEncodeParameters(this.cleanParams(params));

    return `${endpoint}?${query}`;
  }

  logout(): Observable<NbAuthResult> {
     this.window.location.href = this.buildLogoutUrl(); // like the authorizeRedirect method
     return observableOf(new NbAuthResult(true));
  }

// ... rest of strategy

(edit)
Forgot to mention that this requires a callback to invalidate the session locally, or you can make the Token Service available to your Open ID Connect strategy and have it clear the session before redirecting to the Logout endpoint.
(end edit)

As for whether the OAuth2 Strategy should implement that directly, I'd say it shouldn't (logout isn't defined in the OAuth2 RFC tmk). However it also shouldn't passively "succeed", but go back to raising an error if Logout is attempted. If anything else, we could probably enhance Nebular by adding an OpenIDConnect strategy that adds this feature (extending the OAuth2 strategy just as the protocol extends the OAuth2 protocol). Food for thought 🌯🍴😋