Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

rfellows merged PR #8906:
URL: https://github.com/apache/nifi/pull/8906


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

mcgilman commented on code in PR #8906:
URL: https://github.com/apache/nifi/pull/8906#discussion_r1627661463


##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java:
##
@@ -283,11 +285,14 @@ private Handler loadInitialWars(final Set 
bundles) {
 
 // load the web ui app
 final WebAppContext webUiContext = loadWar(webUiWar, 
CONTEXT_PATH_NIFI, frameworkClassLoader);
-webUiContext.getInitParams().put("oidc-supported", 
String.valueOf(props.isOidcEnabled()));
-webUiContext.getInitParams().put("saml-supported", 
String.valueOf(props.isSamlEnabled()));
-webUiContext.getInitParams().put("saml-single-logout-supported", 
String.valueOf(props.isSamlSingleLogoutEnabled()));

Review Comment:
   SAML and OICD are still supported. Previously there were Servlet filters 
which consumed these `init params` which handled this routing. However, in an 
effort to further decouple front end and back end we've removed these Servlet 
Filters here [1]. These `init params` should have been removed in [1] but they 
were overlooked. Currently there is nothing consuming these.
   
   [1] https://github.com/apache/nifi/pull/8843



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

terjekid commented on code in PR #8906:
URL: https://github.com/apache/nifi/pull/8906#discussion_r1627331038


##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java:
##
@@ -283,11 +285,14 @@ private Handler loadInitialWars(final Set 
bundles) {
 
 // load the web ui app
 final WebAppContext webUiContext = loadWar(webUiWar, 
CONTEXT_PATH_NIFI, frameworkClassLoader);
-webUiContext.getInitParams().put("oidc-supported", 
String.valueOf(props.isOidcEnabled()));
-webUiContext.getInitParams().put("saml-supported", 
String.valueOf(props.isSamlEnabled()));
-webUiContext.getInitParams().put("saml-single-logout-supported", 
String.valueOf(props.isSamlSingleLogoutEnabled()));

Review Comment:
   Does it mean that SAML and OICD won't be supported anymore?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

exceptionfactory commented on code in PR #8906:
URL: https://github.com/apache/nifi/pull/8906#discussion_r1625057038


##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java:
##
@@ -412,7 +412,7 @@ private LogoutRequest completeLogoutRequest(final 
HttpServletResponse httpServle
 }
 
 private String getNiFiLogoutCompleteUri() {
-return getNiFiUri() + "logout-complete";
+return getNiFiUri() + "#/logout-complete";

Review Comment:
   Thanks for making the adjustments in `JettyServer` to add the rewrite 
handling @mcgilman, the latest version looks good!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

joewitt commented on PR #8906:
URL: https://github.com/apache/nifi/pull/8906#issuecomment-2146094253

   just built and things are looking really good!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

mcgilman commented on PR #8906:
URL: https://github.com/apache/nifi/pull/8906#issuecomment-2145545377

   > Interested in your thoughts on this.
   
   Thanks for your comments! In my experience there is too much coordination 
between the front and back end to use PathLocationStrategy with dynamic context 
paths. That said, I'm sure we can we could figure it out using the technique 
referenced (where the server transforms the page when served) or possibly 
another approach. HashLocationStrategy completely decouples these concerns as 
the front end route is never even sent to the server.
   
   2.0.0-M3 is released with HashLocationStrategy in place and if there is any 
feedback regarding it we could certainly evaluate the alternative.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

EndzeitBegins commented on PR #8906:
URL: https://github.com/apache/nifi/pull/8906#issuecomment-2145237159

   As far as I remember Angular recommends to use "HTML5 style" routing for 
several years already, here's an exempt [from the 
documentation](https://angular.dev/guide/routing/common-router-tasks#choosing-a-routing-strategy):
   > Almost all Angular projects should use the default HTML5 style. It 
produces URLs that are easier for users to understand and it preserves the 
option to do server-side rendering.
   
   To my knowledge to get this to work properly, one has to set the `baseHref` 
accordingly when serving the `index.html` .
   This does not have to be done at build time, but can be achieved during 
runtime. 
   
   One idea is to use a placeholder as `baseHref` and replace it whenever the 
`index.html` is served. An example of this approach is available [on 
GitHub](https://github.com/mpalourdio/SpringBootAngularHTML5). There might be 
other approaches of course.
   
   I think we should evaluate whether it makes sense to look into this now 
instead of at a later point in time. 
   I think changing the approach later might prove to be more difficult as it 
basically invalides any hash-based links saved by users (or they have to be 
mapped, requiring a long-time workaround).
   
   Also this may avoid the need for a redirect regarding the mapping.
   
   Interested in your thoughts on this. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

joewitt commented on PR #8906:
URL: https://github.com/apache/nifi/pull/8906#issuecomment-2143047480

   guessing yall already on top of this based on discussion above but the build 
fails currently with
   
   `[ERROR]   
OidcLogoutSuccessHandlerTest.testOnLogoutSuccessRequestFoundEndSessionNotSupported:161
 expected:  but was: 

   [ERROR]   
OidcLogoutSuccessHandlerTest.testOnLogoutSuccessRequestFoundEndSessionSupportedTokenFound:212
 expected: 

 but was: 

   [ERROR]   
OidcLogoutSuccessHandlerTest.testOnLogoutSuccessRequestFoundEndSessionSupportedTokenNotFound:177
 expected:  but was: 

   [ERROR]   
OidcLogoutSuccessHandlerTest.testOnLogoutSuccessRequestNotFound:146 expected: 
 but was: 

   [ERROR]   Saml2LogoutSuccessHandlerTest.testOnLogoutSuccessRequestFound:95 
expected:  but was: 

   [ERROR]   
Saml2LogoutSuccessHandlerTest.testOnLogoutSuccessRequestNotFound:79 expected: 
 but was: 
`


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

mcgilman commented on code in PR #8906:
URL: https://github.com/apache/nifi/pull/8906#discussion_r1622961535


##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java:
##
@@ -412,7 +412,7 @@ private LogoutRequest completeLogoutRequest(final 
HttpServletResponse httpServle
 }
 
 private String getNiFiLogoutCompleteUri() {
-return getNiFiUri() + "logout-complete";
+return getNiFiUri() + "#/logout-complete";

Review Comment:
   Must have misread your comment initially but we may be able to manually add 
a Jetty config that just redirects `/nifi/logout-complete` to 
`/nifi/#/logout-complete`. This is a good idea and would address the other 
issue I noted above. Will try it out.  



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

mcgilman commented on code in PR #8906:
URL: https://github.com/apache/nifi/pull/8906#discussion_r1622959963


##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java:
##
@@ -412,7 +412,7 @@ private LogoutRequest completeLogoutRequest(final 
HttpServletResponse httpServle
 }
 
 private String getNiFiLogoutCompleteUri() {
-return getNiFiUri() + "logout-complete";
+return getNiFiUri() + "#/logout-complete";

Review Comment:
   Good point. However, we cannot use that path because we use hash based 
routing. The hash is inserted and all routes begin after it. This allows the 
front end to handle routing independent of the path the application is deployed 
to. Switching to path based routing is a possibility but is a larger effort. 
Path based routing typically requires knowing the base path at build time. 
There may be options for setting it at run time but this would need to be 
investigated more thoroughly. 
   
   I was actually just evaluating this in the context of OIDC and there was an 
issue with hash being encoded. So we'll need to consider the options here. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Re: [PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

exceptionfactory commented on code in PR #8906:
URL: https://github.com/apache/nifi/pull/8906#discussion_r1622948663


##
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java:
##
@@ -412,7 +412,7 @@ private LogoutRequest completeLogoutRequest(final 
HttpServletResponse httpServle
 }
 
 private String getNiFiLogoutCompleteUri() {
-return getNiFiUri() + "logout-complete";
+return getNiFiUri() + "#/logout-complete";

Review Comment:
   This will be a breaking change for existing deployments given that identity 
providers require registration of logout URLs. What do you think about 
considering a redirect from `/nifi/logout-complete` in the Jetty Server 
configuration?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[PR] NIFI-13313: Remove old UI [nifi]

[via GitHub]

mcgilman opened a new pull request, #8906:
URL: https://github.com/apache/nifi/pull/8906

   NIFI-13313:
   - Use nifi-web-frontend as the default UI hosted at /nifi no longer 
deploying nifi-web-ui.
   - Adding logout complete page.
   - Updating backend to redirect to new logout complete page.
   - Remove nifi-web-ui module.
   - Updating LICENSE and NOTICE files for dependencies that are no longer 
included.
   - Removing unnecessary rat config.
   - Updating README.
   - Updating proxy config to mirror actual context path.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org