[WFLY-11584] Legacy Web migrate op fails if a connector has scheme ht…

[tmiyargi]

…tps and no SSL config

Issue: https://issues.jboss.org/browse/WFLY-11584

There is a case were you can have https scheme with no configuration if working with a proxy, https scheme cannot be used to determine if it is using SSL.

[bstansberry]

@fl4via Please review / approve.

Questions I had:

1. Is it a certain thing that an https connector with no ssl child and a proxy-name configured always means this situation? If not should something be added to the 'warnings' node to help point out the ambiguous situation?

2. Setting proxy-address-forwarding=true basically turns on ProxyPeerAddressHandler, which has this in the class javadoc:

 * Handler that sets the peer address to the value of the X-Forwarded-For header.
 * <p>
 * This should only be used behind a proxy that always sets this header, otherwise it
 * is possible for an attacker to forge their peer address;

So is turning this on on the basis of 'proxy-name' having been set sufficient?

[tmiyargi]

@bstansberry, @fl4via, PR amended, everything in the configuration is optional, I cannot set a parameter that might make the user setup insecure in any case, so instead of failing if no SSL configuration is found, there is a warning message and the connector is migrated as http, since that is allowed and probably expected. Let me know if that is fine with you.

[fl4via]

Just to let you know that this is under my radar. I'll have some time slot to look at it later today, it is on top of my todo list.

[fl4via]

I think the migrateConnector if/else statements need some changes.

[fl4via]

I think that if (scheme == null || scheme.equals("http") {...} else if (scheme.equals("https")) {final ModelNode legacyAddOp =...; if (legacyAddOpp == null) {...} else {...}} else {...} is cleaner and leaves no space for bugs. The way it is right now, you are saying that no matter what is the value of scheme (we only know it is not null and not equal to "http", if we have ssl we will go to http. Is that what you intended?

[tmiyargi]

You are right, only http and https values could be used but you can set there the value you like, added the changes. Thank you.

[fl4via]

@bstansberry you can mark this is as ready to merge, IMO

[kabir]

Retest this please