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

AdminFaces redirects over http when using a load balancer #147

Closed
danielemaddaluno opened this issue Jan 2, 2019 · 33 comments
Closed

AdminFaces redirects over http when using a load balancer #147

danielemaddaluno opened this issue Jan 2, 2019 · 33 comments
Labels
Milestone

Comments

@danielemaddaluno
Copy link

danielemaddaluno commented Jan 2, 2019

Issue Overview

admin.loginPage always redirects me over http.

Expected Behaviour

Opening a private page with https should redirect me to the loginPage through https if I'm not logged in.

How to reproduce

My admin-config.properties is like this:

admin.indexPage=index.xhtml
admin.loginPage=/public/sign/signin.xhtml

My AdminSession specialization is like this:

@Named
@SessionScoped
@Specializes
public class LogManager extends AdminSession implements Serializable {
// ...
        @Override
	public boolean isLoggedIn() {
	    return isPartialliLoggedIn() && currentUserProvider != null;
	}
// ...
}

My deployment root is under nights-web-private.
For example if I open this link in https (I deployed for test purpose the ear on cloudjiffy. It will be available online just for a short period):
https://env-2702045.cloudjiffy.net/nights-web-private
I am redirected to
http://env-2702045.cloudjiffy.net/nights-web-private/public/sign/signin.xhtml over a simple http.

Additional Information

I don't know if I'm missing something but it seems like the AdminFilter class inside the doFilter method does not take in account that I'm coming to this page from https. Maybe it's something related to the redirectToLogon method.

  • AdminFaces version: admin-template 1.0.0-RC18
  • PrimeFaces version: PF 6.2
  • JSF implementation: Mojarra 2.2.15
@danielemaddaluno danielemaddaluno changed the title admin.loginPage redirects me over http. admin.loginPage redirects me over http Jan 2, 2019
@danielemaddaluno
Copy link
Author

danielemaddaluno commented Jan 9, 2019

I tried adding this to my web.xml:

<security-constraint>
  <web-resource-collection>
    <web-resource-name>securedapp</web-resource-name>
    <url-pattern>/*</url-pattern>
  </web-resource-collection>
  <user-data-constraint>
    <transport-guarantee>CONFIDENTIAL</transport-guarantee>
  </user-data-constraint>
</security-constraint>

To automatically redirect each http to https.

But after this edit the web page (https://env-2702045.cloudjiffy.net/nights-web-private) through Google Chrome answers like this:

This page isn’t working env-2702045.cloudjiffy.net redirected you too many times.
Try clearing your cookies.
ERR_TOO_MANY_REDIRECTS

@rmpestano
Copy link
Contributor

Hi @danielemaddaluno,

I have just reproduced on admin-starter, accessing the login page directly works on https: https://admin-starter-admin-starter.1d35.starter-us-east-1.openshiftapps.com/admin-starter/login.xhtml

But when I only access the application context it redirects through http and don't work: https://admin-starter-admin-starter.1d35.starter-us-east-1.openshiftapps.com/admin-starter/

I'll have a look, thanks for reporting.

@rmpestano rmpestano added the bug label Jan 9, 2019
@rmpestano rmpestano added this to the 1.0.0 milestone Jan 9, 2019
@danielemaddaluno
Copy link
Author

I don't know if this fact could help you for this bug:
setting the security-constraint in the web.xml (as shown above) gives a redirection loop.

So it seems that somewhere there is an explicit redirect to the http page (otherwise the security-constraint should have solved the problem).
Like https -1-> http -2-> https -1-> http -2-> https -1-> etcetera

Where:

  1. by admin-template
  2. by the security-constraint

Thank you @rmpestano for this library!

rmpestano pushed a commit that referenced this issue Jan 9, 2019
@rmpestano
Copy link
Contributor

Hi, can you try with 1.0.0-RC21-SNAPSHOT?

To use this version you'll need to declare snapshots repository on your pom, see here.

I couldn't test because I'm having problems with openshift deploy.

I hope it helps

rmpestano pushed a commit that referenced this issue Jan 10, 2019
@danielemaddaluno
Copy link
Author

Hi @rmpestano.
Unfortunately I tried again with 1.0.0-RC21-SNAPSHOT but it always redirects me to http.
I redeployed the new ear on cloudjiffy.
Try opening:
https://env-2702045.cloudjiffy.net/nights-web-private

@rmpestano
Copy link
Contributor

rmpestano commented Jan 10, 2019

Can you tell me what's being logged here:

log.info("Configured redirect prefix: "+redirectPrefix);
?

It should be logged only the first time the redirect is done in AdminFilter.

OBS: You may need to enable logging for AdminFilter class.

@danielemaddaluno
Copy link
Author

danielemaddaluno commented Jan 10, 2019

In my logs, even after the redirect, from adminfaces I just see this line:

�[0m�[0m11:30:21,963 INFO [com.github.adminfaces.template.session.AdminServletContextListener] (ServerService Thread Pool -- 206) Using Admin Template 1.0.0-RC21-SNAPSHOT and Admin Theme 1.0.0-RC20

I can't see anything about AdminFilter class in my logs.
Probably I need to enable the logging as you told me...
What should I do for "enable logging for AdminFilter class"?

Thank you for the help.

@rmpestano
Copy link
Contributor

Try using log4j, see example here: https://www.mkyong.com/logging/log4j-hello-world-example/

Just follow the example above through item 3 log4j.properties, on the properties file you can have:

log4j.rootLogger=INFO, stdout

# Redirect log messages to console
log4j.appender.stdout=org.apache.log4j.ConsoleAppender
log4j.appender.stdout.Target=System.out
log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
log4j.appender.stdout.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n

@danielemaddaluno
Copy link
Author

Nothing, still can't see any print in the console.log.
But I think it is configured correctly...

I added an info log in my code to check if it is working and this is printed on the console.

private void logLayout() {
    logger.debug("Configured layout: " + layout);
    logger.info("Configured layout: " + layout);
}

like this:

2019-01-10 13:52:16 INFO LayoutManager:44 - Configured layout: /WEB-INF/templates/template.xhtml

rmpestano pushed a commit that referenced this issue Jan 10, 2019
@rmpestano
Copy link
Contributor

rmpestano commented Jan 10, 2019

Try this on log4j.properties:

log4j.logger.com.github.adminfaces.template.session.AdminFilter=info

Note that the logging of log.info("Configured redirect prefix: "+redirectPrefix); will only happen the first time so you may also need to restart the application.

Also, make sure you are using the latest admin-template snapshot, build the application with the -U flag:
mvn clean package -U .

@danielemaddaluno
Copy link
Author

Still nothing new.
I'm using this log4j.properties:

log4j.rootLogger=INFO, stdout

# Redirect log messages to console
log4j.appender.stdout=org.apache.log4j.ConsoleAppender
log4j.appender.stdout.Target=System.out
log4j.appender.stdout.layout=org.apache.log4j.PatternLayout
log4j.appender.stdout.layout.ConversionPattern=%d{yyyy-MM-dd HH:mm:ss} %-5p %c{1}:%L - %m%n
log4j.logger.com.github.adminfaces.template.session.AdminFilter=info

And these are my last lines on console.log:

[0m2019-01-10 14:09:10 INFO ApplicationInitializer:65 - Using OmniFaces version 2.7
�[0m14:09:10,569 INFO [javax.enterprise.resource.webcontainer.jsf.config] (ServerService Thread Pool -- 302) Initializing Mojarra 2.3.5.SP2 for context '/nights-rest'
�[0m2019-01-10 14:09:10 INFO AdminServletContextListener:23 - Using Admin Template 1.0.0-RC21-SNAPSHOT and Admin Theme 1.0.0-RC20
�[0m14:09:10,699 INFO [javax.enterprise.resource.webcontainer.jsf.config] (ServerService Thread Pool -- 298) Initializing Mojarra 2.3.5.SP2 for context '/nights-web-private'
�[0m�[33m14:09:12,862 WARNING [javax.enterprise.resource.webcontainer.jsf.application] (ServerService Thread Pool -- 298) @FacesConverter is using both value and forClass, only value will be applied.
�[0m�[33m14:09:12,862 WARNING [javax.enterprise.resource.webcontainer.jsf.application] (ServerService Thread Pool -- 298) @FacesConverter is using both value and forClass, only value will be applied.
�[0m�[33m14:09:12,862 WARNING [javax.enterprise.resource.webcontainer.jsf.application] (ServerService Thread Pool -- 298) @FacesConverter is using both value and forClass, only value will be applied.
�[0m�[0m14:09:13,673 INFO [org.primefaces.webapp.PostConstructApplicationEventListener] (ServerService Thread Pool -- 302) Running on PrimeFaces 6.2
�[0m�[0m14:09:13,675 INFO [org.wildfly.extension.undertow] (ServerService Thread Pool -- 302) WFLYUT0021: Registered web context: '/nights-rest' for server 'default-server'
�[0m
2019-01-10 14:09:14 INFO PostConstructApplicationEventListener:40 - Running on PrimeFaces Extensions 6.2.9
�[0m14:09:14,226 INFO [org.wildfly.extension.undertow] (ServerService Thread Pool -- 298) WFLYUT0021: Registered web context: '/nights-web-private' for server 'default-server'
�[0m�[0m14:09:14,235 INFO [org.jboss.as.server] (management-handler-thread - 7) WFLYSRV0010: Deployed "ROOT.ear" (runtime-name : "ROOT.ear")

Plus these lines after the first redirect:

[0m
2019-01-10 14:10:51 INFO MessagesRenderer:135 - autoUpdate attribute is deprecated and will be removed in a future version, use p:autoUpdate component instead.
2019-01-10 14:10:51 INFO MessagesRenderer:135 - autoUpdate attribute is deprecated and will be removed in a future version, use p:autoUpdate component instead.
2019-01-10 14:13:10 INFO MessagesRenderer:135 - autoUpdate attribute is deprecated and will be removed in a future version, use p:autoUpdate component instead.
2019-01-10 14:13:10 INFO MessagesRenderer:135 - autoUpdate attribute is deprecated and will be removed in a future version, use p:autoUpdate component instead.
2019-01-10 14:13:16 INFO LayoutManager:44 - Configured layout: /WEB-INF/templates/template.xhtml
2019-01-10 14:13:16 INFO MessagesRenderer:135 - autoUpdate attribute is deprecated and will be removed in a future version, use p:autoUpdate component instead.
2019-01-10 14:13:16 INFO MessagesRenderer:135 - autoUpdate attribute is deprecated and will be removed in a future version, use p:autoUpdate component instead.

@rmpestano
Copy link
Contributor

Strange it looks ok, maybe the INFO must be in uppercase, see examples here: https://examples.javacodegeeks.com/enterprise-java/log4j/log4j-log-levels-example/

@rmpestano
Copy link
Contributor

rmpestano commented Jan 10, 2019

After digging into this issue I found this (losing the https) is a quite common problem caused by load balancers, e.g see here and here.

In admin-template the issue occours in any response.sendRedirect calls (there are 3 situations) on AdminFilter.

Before this issue we were doing relative redirects e.g sendRedirec(/CONTEXT_PATH+PAGE_TO_REDIRECT) now I'm trying to use full/absolute redirects by saving base url from the request here but it looks like it is not working.

@danielemaddaluno
Copy link
Author

danielemaddaluno commented Jan 10, 2019

Ok now logger prints this in my console.log file:

�[0m2019-01-10 15:57:25 INFO AdminFilter:249 - Configured redirect prefix: http://env-2702045.cloudjiffy.net

rmpestano pushed a commit that referenced this issue Jan 10, 2019
@rmpestano
Copy link
Contributor

rmpestano commented Jan 10, 2019

Can you try again with latest snapshot? Now I'm also looking at server port like spring security does.

@danielemaddaluno
Copy link
Author

danielemaddaluno commented Jan 10, 2019

I deleted from my .m2 folder the 1.0.0-RC21-SNAPSHOT folder and jars just to be sure of reloading the correct one.
But I still get the same:

�[0m2019-01-10 16:28:41 INFO AdminFilter:249 - Configured redirect prefix: http://env-2702045.cloudjiffy.net

Could you add a new log.info with request.getServerPort()?
Probably it would be easier for us to remotely debug, but I never tried to.

rmpestano pushed a commit that referenced this issue Jan 10, 2019
@rmpestano
Copy link
Contributor

Could you add a new log.info with request.getServerPort()?

Done.

@danielemaddaluno
Copy link
Author

I opened as first url this https://env-2702045.cloudjiffy.net/nights-web-private
and got 4 lines in the logs:

2019-01-10 23:37:17 INFO AdminFilter:249 - Configured redirect prefix: http://env-2702045.cloudjiffy.net
2019-01-10 23:37:17 INFO AdminFilter:250 - Server port: 80
2019-01-10 23:37:17 INFO AdminFilter:251 - Is secure: false
2019-01-10 23:37:17 INFO AdminFilter:252 - X-Forwarded-Proto: http

@danielemaddaluno
Copy link
Author

danielemaddaluno commented Jan 10, 2019

Maybe now, just for test purposes, we should try without saving the redirectPrefix inside an AdminFilter variable.

private String getRedirectPrefix(HttpServletRequest request) {
if(redirectPrefix == null) {
String url = request.getRequestURL().toString();
String uri = request.getRequestURI();
int offset = url.indexOf(uri);
redirectPrefix = url.substring(0, offset);
if(request.isSecure()) {
redirectPrefix = redirectPrefix.replace("http:","https:");
}
log.info("Configured redirect prefix: "+redirectPrefix);
}
return redirectPrefix;
}

Replacing the method with something like this that uses a local variable (for redirectPrefix):

private String getRedirectPrefix(HttpServletRequest request) { 
     String redirectPrefix = "";
     String url = request.getRequestURL().toString(); 
     String uri = request.getRequestURI(); 
     int offset = url.indexOf(uri); 
     redirectPrefix = url.substring(0, offset); 
     if(request.isSecure()) {
         log.info("Configured redirect prefix before replace: "+redirectPrefix);
         redirectPrefix = redirectPrefix.replace("http:","https:"); 
     } 
     log.info("Configured redirect prefix: "+redirectPrefix);
     log.info("Server port: "+request.getServerPort());
     log.info("Is secure: "+request.isSecure());
     log.info("X-Forwarded-Proto: "+request.getHeader("X-Forwarded-Proto"));
     return redirectPrefix; 
 } 

If the redirectPrefix is badly set the first time will be always marked with http.
This way we can check if it prints the same thing for each request I will try to do.

@danielemaddaluno
Copy link
Author

I just noticed that passing from 1.0.0-RC18
to 1.0.0-RC21-SNAPSHOT make the primefaces component p:schedule stop working.

@rmpestano
Copy link
Contributor

Hi, do you see the same problem on the showcase page? http://admin-showcase-admin-showcase.7e14.starter-us-west-2.openshiftapps.com/showcase/pages/components/schedule.xhtml

Have you cleared browser cache?

@danielemaddaluno
Copy link
Author

Can't see that page now, I'll open another issue for this thing, I thought that was related with this 1.0.0-RC21-SNAPSHOT but I don't think so now.

@rmpestano
Copy link
Contributor

Hi @danielemaddaluno, about the redirect problem I think we will have to force https in some way because all mechanisms to identify the protocol are failing. I really think it is a balancer configuration issue but sometimes we don't have access/permissions to change that config.

By forcing the protocol I'm thinking in using a system property or env variable named admin.protocol=https so when you run the application on a secure env you can pass -Dadmin.protocol=https.

I've tested in openshift and it worked, see here: https://admin-admin-starter.1d35.starter-us-east-1.openshiftapps.com/admin-starter/

On openshift I've added the property on the deployment configuration:
screenshot from 2019-01-13 08-56-06

@rmpestano
Copy link
Contributor

Latest admin-template snasphot has the admin.protocol mechanism

@rmpestano
Copy link
Contributor

rmpestano commented Jan 13, 2019

Also, one thing that I noticed is that on openshift the x-forward-proto is being send properly so in addition I'll also check that header to decide if we'll use https:

screenshot from 2019-01-13 09-09-29

rmpestano pushed a commit that referenced this issue Jan 13, 2019
@danielemaddaluno
Copy link
Author

It would be nice to be able to set it both from env variables and from resources/admin-config.properties file.
Putting the admin.protocol=https in wildfly env variables makes it work.
screenshot 2019-01-13 16 48 39

However putting this code below in the web.xml still gives me a looping redirect problem. I don't know if we could do something for this.
I used this to force all http to https.

<security-constraint>
	<web-resource-collection>
		<web-resource-name>securedapp</web-resource-name>
		<url-pattern>/*</url-pattern>
	</web-resource-collection>
	<user-data-constraint>
		<transport-guarantee>CONFIDENTIAL</transport-guarantee>
	</user-data-constraint>
</security-constraint>

Let me know I there exist some cheats for this thing too.

Thank you again and again @rmpestano

@rmpestano
Copy link
Contributor

"It would be nice to be able to set it both from env variables and from resources/admin-config.properties file"

I think forcing the protocol in admin-config.properties will make the application stop to work locally on dev machine or e.g in pre-production where sometimes we don't have https. It makes sense for you?

I'll take a look on the security-constraint config.

@danielemaddaluno
Copy link
Author

Yeah you are right but if it was possible to set https and a specific port I would use that settings on my local dev machine without problems.

On my local dev machine I can open both:

  1. http://localhost:8080/nights-web-private/
  2. https://localhost:8443/nights-web-private/

I'm using Wildfly with Spring Tools 3 for Eclipse.
In my local machine using the security-constraint config I am automatically redirected from 1 to 2, on remote (cloudjiffy) I have that redirect loop.

@rmpestano
Copy link
Contributor

I think It is not a good idea to have infrastructure configuration in admin-config. I will have a look into the redirect loop asap.

@rmpestano
Copy link
Contributor

With the <security-constraint> configuration I don't have redirect loop on openshift, it redirects to 8443 and doesn't find the page:

screenshot from 2019-01-13 20-19-47

Looking at the logs it doesn't reach AdminFilter because I see nothing on logs.

locally it is working:

starter-local

Again, it looks like a infrastructure thing rather then adminfaces, see here: https://developer.jboss.org/thread/272606?_sscc=t

Do you have other (JavaEE) applications using this setup () working on the cloud without issues?

@rmpestano
Copy link
Contributor

Do you have other (JavaEE) applications using this setup () working on the cloud without issues?

Or you can just disable admin filter, in admin-config.properties, to see if the application still works:

admin.disableFilter=true

@danielemaddaluno
Copy link
Author

Sorry for being late in the answer.
I was trying a lot of different configuration to force http to https, just to add something more here.
Your commits fixed the initial problem so I think we could close the issue.

I first tried these two solutions to force http to https:
https://jelastic.zendesk.com/hc/en-us/community/posts/206121996-HTTP-HTTPS-redirection-into-the-Tomcat
Both these solutions gave me a redirect loop... Still can't understand why, and probably it is not related to your lib as you previously stated, however I opened a ticket on cloudjiffy to understand this better.

However just for the sake of knowledge I finally solved the redirect problem adding a layer of "Nginx load balancer" adding this redirect rule as stated here:

# force https-redirects
if ($http_X_Forwarded_Proto = http) {
    return 302 https://$host$request_uri;
}

Thank you for the patience @rmpestano

@rmpestano
Copy link
Contributor

Great, also thank you for the interaction here, for sure AdminFaces wasn't working with any balancer before due to it's relative sendRedirects in AdminFilter.

@rmpestano rmpestano changed the title admin.loginPage redirects me over http AdminFaces redirects over http when using a load balancer Jan 14, 2019
@rmpestano rmpestano modified the milestones: 1.0.0, 1.0.0-RC21 Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants