Skip to content
This repository has been archived by the owner on Jan 24, 2023. It is now read-only.

Commit

Permalink
Always return code 200 in session verification endpoint (#4622)
Browse files Browse the repository at this point in the history
  • Loading branch information
ikapelyukhin committed Oct 16, 2020
1 parent d857b19 commit ce4c108
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class NotSetupGuardService implements CanActivate {

canActivate(): Observable<boolean> {

const url = `/pp/${proxyAPIVersion}/auth/session/verify`;
const url = `/api/${proxyAPIVersion}/auth/verify`;
return this.http.get(url).pipe(
map(v => {
// If the requests succeeds, then the user has a session, so everything must be setup already
Expand Down
26 changes: 17 additions & 9 deletions src/frontend/packages/store/src/effects/auth.effects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { DispatchOnlyAppState } from '../app-state';
import { BrowserStandardEncoder } from '../browser-encoder';
import { getDashboardStateSessionId } from '../helpers/store-helpers';
import { stratosEntityCatalog } from '../stratos-entity-catalog';
import { SessionData } from '../types/auth.types';
import { SessionData, SessionDataEnvelope } from '../types/auth.types';

const SETUP_HEADER = 'stratos-setup-required';
const UPGRADE_HEADER = 'retry-after';
Expand Down Expand Up @@ -73,19 +73,27 @@ export class AuthEffect {
'x-cap-request-date': (Math.floor(Date.now() / 1000)).toString()
};

return this.http.get<SessionData>('/pp/v1/auth/session/verify', {
return this.http.get<SessionDataEnvelope>('/api/v1/auth/verify', {
headers,
observe: 'response',
withCredentials: true,
}).pipe(
mergeMap(response => {
const sessionData = response.body;
sessionData.sessionExpiresOn = parseInt(response.headers.get('x-cap-session-expires-on'), 10) * 1000;
this.rehydrateDashboardState(this.store, sessionData);
return [
stratosEntityCatalog.systemInfo.actions.getSystemInfo(true),
new VerifiedSession(sessionData, action.updateEndpoints)
];
const envelope = response.body;
if (envelope.status === 'error') {
const ssoOptions = response.headers.get(SSO_HEADER) as string;
// Check for cookie domain mismatch with the requesting URL
const isDomainMismatch = this.isDomainMismatch(response.headers);
return action.login ? [new InvalidSession(false, false, isDomainMismatch, ssoOptions)] : [new ResetAuth()];
} else {
const sessionData = envelope.data;
sessionData.sessionExpiresOn = parseInt(response.headers.get('x-cap-session-expires-on'), 10) * 1000;
this.rehydrateDashboardState(this.store, sessionData);
return [
stratosEntityCatalog.systemInfo.actions.getSystemInfo(true),
new VerifiedSession(sessionData, action.updateEndpoints)
];
}
}),
catchError((err, caught) => {
let setupMode = false;
Expand Down
7 changes: 7 additions & 0 deletions src/frontend/packages/store/src/types/auth.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,13 @@ export interface SessionData {
};
config: SessionDataConfig;
}

export interface SessionDataEnvelope {
status: string;
error?: string;
data?: SessionData;
}

export interface Diagnostics {
deploymentType?: string;
gitClientVersion?: string;
Expand Down
44 changes: 15 additions & 29 deletions src/jetstream/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (

"github.com/cloudfoundry-incubator/stratos/src/jetstream/crypto"
"github.com/cloudfoundry-incubator/stratos/src/jetstream/repository/interfaces"
"github.com/labstack/echo/v4"
. "github.com/smartystreets/goconvey/convey"
)

Expand Down Expand Up @@ -786,7 +785,7 @@ func TestVerifySession(t *testing.T) {

var expectedScopes = `"scopes":["openid","scim.read","cloud_controller.admin","uaa.user","cloud_controller.read","password.write","routing.router_groups.read","cloud_controller.write","doppler.firehose","scim.write"]`

var expectedBody = `{"version":{"proxy_version":"dev","database_version":20161117141922},"user":{"guid":"asd-gjfg-bob","name":"admin","admin":false,` + expectedScopes + `},"endpoints":{"cf":{}},"plugins":null,"config":{"enableTechPreview":false,"APIKeysEnabled":"admin_only"}}`
var expectedBody = `{"status":"ok","error":"","data":{"version":{"proxy_version":"dev","database_version":20161117141922},"user":{"guid":"asd-gjfg-bob","name":"admin","admin":false,` + expectedScopes + `},"endpoints":{"cf":{}},"plugins":null,"config":{"enableTechPreview":false,"APIKeysEnabled":"admin_only"}}}`

Convey("Should contain expected body", func() {
So(res, ShouldNotBeNil)
Expand All @@ -809,7 +808,7 @@ func TestVerifySessionNoDate(t *testing.T) {
"username": "admin",
"password": "changeme",
})
_, _, ctx, pp, db, _ := setupHTTPTest(req)
res, _, ctx, pp, db, _ := setupHTTPTest(req)
defer db.Close()

// Set a dummy userid in session - normally the login to UAA would do this.
Expand All @@ -824,20 +823,16 @@ func TestVerifySessionNoDate(t *testing.T) {
})

err := pp.verifySession(ctx)
Convey("Should fail to verify session.", func() {

So(err, ShouldNotBeNil)
Convey("Should not fail to verify session.", func() {
So(err, ShouldBeNil)
})

errHTTP, ok := err.(*echo.HTTPError)
Convey("should be able to cast error to HTTPError", func() {
So(ok, ShouldBeTrue)
var expectedBody = `{"status":"error","error":"Could not find session date","data":null}`
Convey("Should contain expected body", func() {
So(res, ShouldNotBeNil)
So(strings.TrimSpace(res.Body.String()), ShouldEqual, expectedBody)
})

var expectedCode = 403
Convey("Request should have expected code", func() {
So(errHTTP.Code, ShouldEqual, expectedCode)
})
})

}
Expand All @@ -851,7 +846,7 @@ func TestVerifySessionExpired(t *testing.T) {
"username": "admin",
"password": "changeme",
})
_, _, ctx, pp, db, mock := setupHTTPTest(req)
res, _, ctx, pp, db, mock := setupHTTPTest(req)
defer db.Close()

if e := pp.InitStratosAuthService(interfaces.Remote); e != nil {
Expand All @@ -877,24 +872,15 @@ func TestVerifySessionExpired(t *testing.T) {
AddRow(mockUAAToken, mockUAAToken, sessionValues["exp"], false))
err := pp.verifySession(ctx)

Convey("Should fail to verify session", func() {
So(err, ShouldNotBeNil)
Convey("Should not fail to verify session", func() {
So(err, ShouldBeNil)
})

errHTTP, ok := err.(*echo.HTTPError)
Convey("should be able to cast error to HTTPError", func() {
So(ok, ShouldBeTrue)
})

var expectedCode = 403
Convey("Request should have expected code", func() {
So(errHTTP.Code, ShouldEqual, expectedCode)
var expectedBody = `{"status":"error","error":"Could not verify user","data":null}`
Convey("Should contain expected body", func() {
So(res, ShouldNotBeNil)
So(strings.TrimSpace(res.Body.String()), ShouldEqual, expectedBody)
})
//
//Convey("Should meet expectations", func() {
// So(mock.ExpectationsWereMet(), ShouldBeNil)
//})

})

}
6 changes: 3 additions & 3 deletions src/jetstream/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -957,6 +957,9 @@ func (p *portalProxy) registerRoutes(e *echo.Echo, needSetupMiddleware bool) {

staticDir, staticDirErr := getStaticFiles(p.Env().String("UI_PATH", "./ui"))

// Verify Session
e.GET("/api/v1/auth/verify", p.verifySession)

// Always serve the backend API from /pp
pp := e.Group("/pp")

Expand Down Expand Up @@ -1026,9 +1029,6 @@ func (p *portalProxy) registerRoutes(e *echo.Echo, needSetupMiddleware bool) {
// Connect to Endpoint (SSO)
sessionAuthGroup.GET("/tokens", p.ssoLoginToCNSI)

// Verify Session
sessionAuthGroup.GET("/session/verify", p.verifySession)

// Info
sessionGroup.GET("/info", p.info)

Expand Down
42 changes: 22 additions & 20 deletions src/jetstream/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,26 @@ func (p *portalProxy) sessionMiddleware() echo.MiddlewareFunc {
return p.sessionMiddlewareWithConfig(MiddlewareConfig{})
}

func (p *portalProxy) clearSessionCookie(c echo.Context, setCookieDomain bool) {
if setCookieDomain {
// Tell the frontend what the Cookie Domain is so it can check if sessions will work
// (used in verifySession)
c.Response().Header().Set(StratosDomainHeader, p.Config.CookieDomain)
}

// Clear any session cookie
cookie := new(http.Cookie)
cookie.Name = p.SessionCookieName
cookie.Value = ""
cookie.Expires = time.Now().Add(-24 * time.Hour)
cookie.Domain = p.SessionStoreOptions.Domain
cookie.HttpOnly = p.SessionStoreOptions.HttpOnly
cookie.Secure = p.SessionStoreOptions.Secure
cookie.Path = p.SessionStoreOptions.Path
cookie.MaxAge = 0
c.SetCookie(cookie)
}

func (p *portalProxy) sessionMiddlewareWithConfig(config MiddlewareConfig) echo.MiddlewareFunc {
// Default skipper function always returns false
if config.Skipper == nil {
Expand All @@ -115,26 +135,8 @@ func (p *portalProxy) sessionMiddlewareWithConfig(config MiddlewareConfig) echo.
return h(c)
}

// Don't log an error if we are verifying the session, as a failure is not an error
isVerify := strings.HasSuffix(c.Request().RequestURI, "/auth/session/verify")
if isVerify {
// Tell the frontend what the Cookie Domain is so it can check if sessions will work
c.Response().Header().Set(StratosDomainHeader, p.Config.CookieDomain)
}

// Clear any session cookie
cookie := new(http.Cookie)
cookie.Name = p.SessionCookieName
cookie.Value = ""
cookie.Expires = time.Now().Add(-24 * time.Hour)
cookie.Domain = p.SessionStoreOptions.Domain
cookie.HttpOnly = p.SessionStoreOptions.HttpOnly
cookie.Secure = p.SessionStoreOptions.Secure
cookie.Path = p.SessionStoreOptions.Path
cookie.MaxAge = 0
c.SetCookie(cookie)

return handleSessionError(p.Config, c, err, isVerify, "User session could not be found")
p.clearSessionCookie(c, false)
return handleSessionError(p.Config, c, err, false, "User session could not be found")
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/jetstream/mock_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,13 @@ func setupPortalProxy(db *sql.DB) *portalProxy {
initialisedEndpoint := initCFPlugin(pp)
pp.Plugins = make(map[string]interfaces.StratosPlugin)
pp.Plugins["cf"] = initialisedEndpoint

pp.SessionStoreOptions = new(sessions.Options)
pp.SessionStoreOptions.Domain = "example.org"
pp.SessionStoreOptions.HttpOnly = false
pp.SessionStoreOptions.Secure = false
pp.SessionStoreOptions.Path = "/"

return pp
}

Expand Down
6 changes: 3 additions & 3 deletions src/jetstream/repository/interfaces/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ type ErrHTTPRequest struct {
Response string
}

// ErrorResponseBody -- struct for generating bodies of error responses
type ErrorResponseBody struct {
Error string `json:"error"`
Status string `json:"status"`
Error string `json:"error"`
}

func (e ErrHTTPShadow) Error() string {
Expand All @@ -38,7 +38,7 @@ func NewHTTPShadowError(status int, userFacingError string, fmtString string, ar
//Only set the HTTPError field of the ErrHTTPShadow struct if we have been passed an error message intended for logging
httpErrorMsg := ""
if len(userFacingError) > 0 {
errBody := ErrorResponseBody{Error: userFacingError}
errBody := ErrorResponseBody{Status: "error", Error: userFacingError}
bytes, _ := json.Marshal(errBody)
httpErrorMsg = string(bytes)
}
Expand Down
Loading

0 comments on commit ce4c108

Please sign in to comment.