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

SPAs - redirect to login page if unauthorized. (#4554) #4585

Merged

Conversation

Projects
None yet
3 participants
@varshavaradarajan
Copy link
Contributor

commented Mar 29, 2018

No description provided.

@varshavaradarajan

This comment has been minimized.

Copy link
Contributor Author

commented Apr 3, 2018

Closing this as upon logging in, the url it goes to is /go/api/dashboard. Not sure how to block some urls from being remembered in the session. @ketan said he'll take care of it with the new security implementation as part of the spring security upgrade. Can be reopened if needed.

@ketan

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

This PR is still relevant. What needs fixing is the server side "remember url" logic.

@ketan ketan reopened this Apr 3, 2018

@@ -207,6 +213,12 @@ CrudMixins.AllOperations = function (operations, options, overrides = {}) {
});
};

const redirectToLoginPageIfUnauthorized = function(jqXHR) {

This comment has been minimized.

Copy link
@ketan

ketan Apr 3, 2018

Member

Since we're using $.ajax. We can probably add a global handler, if you like. http://api.jquery.com/category/ajax/global-ajax-event-handlers/.

@ketan

This comment has been minimized.

Copy link
Member

commented Apr 3, 2018

Also — this might be relevant —

<property name="urlPatternsThatShouldNotBeRedirectedToAfterLogin" value="(\.json)|(\?.*format=json)|(/images/)|(\.css)|(\.ico)|(\.js)|(/auth/login)|(/auth/logout)"/>

@varshavaradarajan varshavaradarajan force-pushed the varshavaradarajan:spas-redirect-to-login-page branch 2 times, most recently from 6384d6f to a5a909b Apr 5, 2018

@@ -80,6 +80,10 @@ private AuthenticationEntryPoint determineAuthenticationPoint(HttpServletRequest
return point;
}

private boolean isAnApiRequest(HttpServletRequest httpRequest) {
return httpRequest.getRequestURI().contains("/api/");

This comment has been minimized.

Copy link
@ketan

ketan Apr 5, 2018

Member

I'd probably err on the side of doing a startsWith to avoid situations where you're accessing files/:pipeline_name/:pipeline_counter/:stage_name/:stage_counter/:job_name/foo/api/index.html

@@ -43,7 +43,7 @@
/agent-websocket/**=modeAwareFilter,artifactSizeEnforcementFilter,i18nlocaleResolver,httpSessionContextIntegrationFilter,x509ProcessingFilter,x509AccessDenied,agentRemotingFilterInvocationInterceptor

/cctray.xml=modeAwareFilter,i18nlocaleResolver,httpSessionContextIntegrationFilter,apiSessionFilter,goLogoutFilter,removeAdminPermissionFilter,oauthProcessingFilter,basicProcessingFilter,authenticationProcessingFilter,reAuthenticationFilter,userEnabledCheckFilter,anonymousProcessingFilter,basicAuthenticationAccessDenied,denyGoCDAccessForArtifactsFilter,sessionFixationProtectionFilter,filterInvocationInterceptor,flashLoader,urlRewriter
/api/**=modeAwareFilter,i18nlocaleResolver,httpSessionContextIntegrationFilter,apiSessionFilter,goLogoutFilter,removeAdminPermissionFilter,oauthProcessingFilter,basicProcessingFilter,authenticationProcessingFilter,reAuthenticationFilter,userEnabledCheckFilter,anonymousProcessingFilter,basicAuthenticationAccessDenied,denyGoCDAccessForArtifactsFilter,sessionFixationProtectionFilter,filterInvocationInterceptor,flashLoader,urlRewriter
/api/**=modeAwareFilter,i18nlocaleResolver,httpSessionContextIntegrationFilter,apiSessionFilter,goLogoutFilter,removeAdminPermissionFilter,oauthProcessingFilter,basicProcessingFilter,authenticationProcessingFilter,reAuthenticationFilter,userEnabledCheckFilter,anonymousProcessingFilter,basicAuthenticationAccessDenied,cruiseLoginOrBasicAuthentication,denyGoCDAccessForArtifactsFilter,sessionFixationProtectionFilter,filterInvocationInterceptor,flashLoader,urlRewriter

This comment has been minimized.

Copy link
@ketan

ketan Apr 5, 2018

Member

Was removing this intentional?

This comment has been minimized.

Copy link
@varshavaradarajan

varshavaradarajan Apr 5, 2018

Author Contributor

What do you mean? I added cruiseLoginOrBasicAuthentication to api/**, didn't remove anything.

@varshavaradarajan varshavaradarajan force-pushed the varshavaradarajan:spas-redirect-to-login-page branch 2 times, most recently from f41df56 to 055bff4 Apr 6, 2018

@varshavaradarajan varshavaradarajan changed the title [WIP] SPAs - redirect to login page if unauthorized. (#4554) SPAs - redirect to login page if unauthorized. (#4554) Apr 6, 2018

@yankurk

This comment has been minimized.

Copy link
Contributor

commented Apr 6, 2018

I tested the functionality locally. It works fine.

SPAs - redirect to login page if unauthorized. (#4554)
* Set the status as 401 for an api request.

@varshavaradarajan varshavaradarajan force-pushed the varshavaradarajan:spas-redirect-to-login-page branch from 055bff4 to eb15ae0 Apr 6, 2018

@varshavaradarajan varshavaradarajan merged commit e901708 into gocd:master Apr 6, 2018

1 check passed

license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.