Project

General

Profile

Feature #3931

single sign-on for virtual desktop mode

Added by Greg Shah about 5 years ago. Updated 3 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
version:

hotel_gui_add_reservation_virtual.png (153 KB) Greg Shah, 04/13/2023 10:28 AM

hotel_gui_integration_embedded.png (1.11 MB) Greg Shah, 04/13/2023 10:29 AM

osuseredit.png (59.1 KB) Galya B, 08/04/2023 04:10 AM

fwduseredit.png (88.4 KB) Galya B, 08/04/2023 04:10 AM

osusersscreen.png (7.52 MB) Galya B, 08/04/2023 04:10 AM

webres.zip (6.54 KB) Galya B, 09/01/2023 09:14 AM

fwd-webres.zip (37.7 KB) Galya B, 09/04/2023 11:28 AM

1.desktop-2560.png (34.7 KB) Galya B, 09/04/2023 11:31 AM

2.desktop-2560-theme.png (41.5 KB) Galya B, 09/04/2023 11:31 AM

3.desktop-2560-theme-error.png (51.9 KB) Galya B, 09/04/2023 11:31 AM

4.ipad-mini-portrait.png (47.4 KB) Galya B, 09/04/2023 11:31 AM

5.ipad-mini-landscape.png (47 KB) Galya B, 09/04/2023 11:31 AM

6.iphone-se-portrait.png (40 KB) Galya B, 09/04/2023 11:31 AM

7.iphone-se-landscape.png (29.6 KB) Galya B, 09/04/2023 11:31 AM

osusersmanagement.png (49.1 KB) Galya B, 09/07/2023 03:15 AM

alert-deleting-osuser-with-mapping.png (88.4 KB) Galya B, 09/07/2023 07:54 AM

login-in-Dutch.png (62.3 KB) Galya B, 09/15/2023 06:48 AM

tr.diff Magnifier (7.25 KB) Hynek Cihlar, 09/15/2023 07:01 AM

HotelGuiSsoAuthenticator.java Magnifier (9.51 KB) Galya B, 10/05/2023 12:23 PM


Related issues

Related to User Interface - Feature #4129: map web client users to OS accounts Closed
Related to Runtime Infrastructure - Feature #4406: server-side REST execution without appserver agents New
Related to User Interface - Feature #5784: multi-factor authentication New
Related to User Interface - Feature #7814: Supporting translated texts for web login / logout screens New
Related to User Interface - Feature #7884: Standard 4GL methods used for login screen authentication to be exposed in sessionless helper classes Review
Related to User Interface - Bug #8074: Preferred UI Theme not selected in web with auto-login and forked sessions Closed

History

#1 Updated by Greg Shah about 5 years ago

Although virtual desktop mode was initially conceived to be primary for testing purposes, multiple customers have chosen to use it as their default deployment choice. This ensures no need to retrain users or ask users to learn something new if they have no interest.

Embedded mode is then the mode for users which want an upgraded, more modern UI.

Since this seems to be a trend, we've also heard from customers that they want to eliminate the need for both an OS-level login as well as the application-level login. This task is meant to deliver that feature.

Please do consider that Linux and Windows have differences in authentication and spawning. Those differences may be meaningful in this task. Although we want the same support on all platforms, it is more important is to ensure that Linux has a solution in this regard.

Avoiding the need to implement user-level accounts in the operating system would be a very good result, because it would eliminate some complexity from deployments. On the other hand, it is usually important to run each user in their own OS account so that they are isolated from a security perspective. Reconciling these two interests is the largest challenge here. For this reason, the #4129 task is needed as part of this work.

This work is related to #3770-81 (embedded mode changes for login) and #4129 (mapping FWD sessions to OS users).

#3 Updated by Greg Shah over 4 years ago

  • Related to Feature #4129: map web client users to OS accounts added

#4 Updated by Greg Shah about 4 years ago

  • Related to Feature #4406: server-side REST execution without appserver agents added

#6 Updated by Constantin Asofiei over 2 years ago

This is dependent on the auth hook mentioned in #3770-81 (auth hook to be executed before spawning the FWD client) and #4129 (user mapping between legacy users and OS users).

#4129 I think can be solved with a directory configuration to map OS users to application-level users (while still having a default OS user if no mapping exists). The problem here is that we should add the Admin Console support for this, too, otherwise the FWD server needs to be restarted, to add a new application (not FWD) user. I don't think we need to map FWD users for application users (we can still use the default FWD user).

For #3770-81, the simplest way is to provide a directory-configured auth hook which checks the credentials using some custom 4GL-like Java code (i.e. check some table for the user/password pair). If the hook allows the credentials, then the spawner can proceed directly to spawn the Web client, using the OS user configured in #4129, without any password. For customer applications, we would need some input on how this hook will look like (or just the 4GL program which we would use as a 'template'); and determine what application-level code needs to change, so that the p2j-entry program will bypass the login dialog (or just a fully new p2j-entry program). And a way to pass this user to the p2j-entry program, so the application can initialize using this user's permissions, etc. - this can be done by passing a UUID to the p2j-entry program, and have some new FWD-extension API to get the application user from that UUID, so the application:
  1. knows that the user was validated with the auth took.
  2. can initialize the client using that user.

Considering that the UUID most likely will be passed to the FWD client as a bootstrap config, we could always add this information at the FWD session, and just provide a FWD extension function which gets the application user set at the current context.

We can't pass directly the username, as the same username can be used in multiple browsers, and we don't want someone find the 'sweet spot' and hijack the period between auth hook and p2j-entry call.

So we need these two to make #3931 work.

#7 Updated by Greg Shah over 2 years ago

#8 Updated by Greg Shah over 2 years ago

For #3770-81, the simplest way is to provide a directory-configured auth hook which checks the credentials using some custom 4GL-like Java code (i.e. check some table for the user/password pair).

Agreed. I would add these thoughts:

  • We should allow for this to be "real" converted 4GL code, not just hand written Java code. I think most customers will find it easiest to extract their existing authentication code and put it into a 4GL procedure.
  • The authentication hook needs to have a standardized 4GL signature. Within reason, we should plan for the various use cases and ensure the signature is robust enough to handle them all.
  • We definitely want to have a single approach for both Embedded Mode and Virtual Desktop Mode, if possible.
  • The original discussions with the customer (for which this task was defined) had the idea that the authentication hook would return an application-generated token which it would then use for matching up with the account when the actual FWD client (and session) is created. It is not clear to me if we need to do this at the application level or if we can somehow handle it ourselves (e.g. through something like the CLIENT-PRINCIPAL). I also wonder if this needs to be different from the one-time use token that we already create when we redirect from the login screen to the FWD client's embedded web environment.
  • Should this hook also include the option to allow the 4GL code to map the application account to an OS account (and FWD account)? This would allow each application to define their own policies. On the down side, it is more code to manage and it encodes some implementation details into the 4GL hook, which isn't great.
  • I want to plan for multi-factor authentication here as well (see #5784). I am not proposing implementing multi-factor authentication as part of this task. I just want to plan for it so that it can be easily added later. For MFA/2FA support, I want as much of it in the FWD part of the implementation as possible, so that each customer does not have to do much to add it.

I am also wondering how we can plan ahead for a Swing GUI based standard login that can be used with the same facilities (auth hook, MFA...). Likewise we should consider how our existing ChUI based authentication hooks might integrate with this facility. In a perfect world we would not have 2 different ways to implement auth hooks and MFA.

#10 Updated by Galya B over 1 year ago

Support for <customers_large_gui_application> user name replacing OS account name in log files (as requested in #4331) should be covered by this task.

#13 Updated by Galya B about 1 year ago

What is virtual desktop mode?

#14 Updated by Constantin Asofiei about 1 year ago

Galya Bogdanova wrote:

What is virtual desktop mode?

The FWD Web clients; in GUI mode, there is a virtual desktop where application windows are displayed. In ChUI mode, there is just a terminal window emulated.

#15 Updated by Galya B about 1 year ago

Can you give me pointers on what parts of the application are currently isolated by the OS users (maybe a few classes or wikis to read through)?

#16 Updated by Galya B about 1 year ago

Are these all the resources that are OS user specific:

      "logon",
      "context",
      "change",
      "shutdown",
      "debug",
      "accounts",
      "extensions",
      "admin" 

#17 Updated by Greg Shah about 1 year ago

Virtual Desktop Mode:

Notice how we create a kind of "desktop" in the browser tab. We have our own taskbar to switch between windows, each window can be minimized/restored/maximized... each window has all of its "chrome" or "decorations" like a border, titlebar and so forth. This provides a compatible approach that is easy to use for migrating your existing users.

Compare that with Embedded Mode:

Each legacy 4G: "screen" is cut down (the window chrome/decorations are removed) and we render in an iframe. This can be put into a larger web application. That web app can do anything and is just normal/modern web code. It can control the iframe with a javascript API and it can "up call" to invoke converted 4GL logic in the FWD server, getting back data as needed. This provides a way to leverage all the converted screens while also writing a new modern web UI for the high value screens. It allows evolution to a modern approach without havving to rewrite 90% of your screens.

#18 Updated by Greg Shah about 1 year ago

Can you give me pointers on what parts of the application are currently isolated by the OS users (maybe a few classes or wikis to read through)?

The isolation is not explicit, but is implicitly there based on the OS process in which something runs. The FWD server has an OS account in which it is started, so anything running on the server implicitly uses that account.

Each FWD client process is started in a specific OS account and thus anything that runs on the client is implicitly using that account. It is one of the primary reasons the FWD client process exists and all of the Client Platform Delegate functionality runs there.

In #4065 we are allowing the optional execution of client-platform processing in the server process. This can only be done in cases where the OS account does not matter.

#19 Updated by Greg Shah about 1 year ago

Galya Bogdanova wrote:

Are these all the resources that are OS user specific:
[...]

None of them relate to OS accounts. All of these things are FWD SecurityManager resources, which don't relate to OS accounts.

#20 Updated by Galya B about 1 year ago

Constantin, in p2j.emain.js there is a function me.login - I can't find how / where it's executed. There is p2j.embedded.invokeProcedure(null, "doLogin", args, null); in emain.pageLoaded that probably is the call I'm looking for, but then I can't find a call to emain.pageLoaded outside emain.login. I see dojo is used and I'm not familiar with the lib, but at least if you can demystify this one, it will be helpful.

#21 Updated by Galya B about 1 year ago

As far as I understand virtual desktop allows access to an app through directory.xml permissions linked to OS users. What is the OE equivalent? Are their web apps protected by the OS login or directly accessible for internet access and authentication is dealt with on an app level?

#22 Updated by Greg Shah about 1 year ago

As far as I understand virtual desktop allows access to an app through directory.xml permissions linked to OS users.

Not always. We do have a way to force a specific OS user to be used for spawning, but by default we prompt the user at an initial login page which is sent by the FWD server.

What is the OE equivalent? Are their web apps protected by the OS login or directly accessible for internet access and authentication is dealt with on an app level?

There is no equivalent. They have no web version of GUI. You can only ever run GUI as a Windows Dewsktop application. To provide remote access, most customers use something like Windows Terminal Server.

#23 Updated by Galya B about 1 year ago

Greg Shah wrote:

To provide remote access, most customers use something like Windows Terminal Server.

This sounds like they are also using an additional layer of authentication, but pre-configured and automatically executed. There seems to be many ways to configure auth in Terminal Server. I guess they have at least predefined usernames.

Client Connection

After the user types a username and password, packets are sent encrypted to the Terminal Server. The Winlogon process then performs the necessary account authentication to ensure that the user has privilege to log on and passes the user's domain and username to the Terminal Server service, which maintains a domain/username SessionID list.

So this is still configured in Winlogon with OS users.

When their app is accessed through Windows Terminal Server, does it still load the login screen?

#24 Updated by Greg Shah about 1 year ago

Ignore the OE nonsense with Terminal Server. Terminal Server itself has nothing to do with OE. My point is just that OE only provides a Windows Desktop application, they have no web client in the way that we have one.

Our solution provides a major upgrade in that all of the original application screens are available in the browser and we don't do any remote desktop nonsense.

However, do to the need to have a FWD client for each session, we need to run that client in the correct OS account. OE doesn't have this problem because it knows that you have already logged in to Windows before you ever launch the Windows Desktop GUI app written in 4GL. In FWD, we must spawn these clients from the web server. That spawning means we need knowledge of the OS account in order to create the new FWD client process. Thus we have an extra OS login step that doesn't exist in OE.

#25 Updated by Galya B about 1 year ago

I'm not looking for the solution yet. I want to get the whole picture and link the fragments from all related tasks, so the question might not have been even in the right task. As far as I can see it at the moment, there are some gaps or questions in the related task descriptions, so I want to make sure I can offer a well rounded solution before starting any implementation.

#26 Updated by Greg Shah about 1 year ago

I'm not looking for the solution yet. I want to get the whole picture and link the fragments from all related tasks, so the question might not have been even in the right task. As far as I can see it at the moment, there are some gaps or questions in the related task descriptions, so I want to make sure I can offer a well rounded solution before starting any implementation.

I understand. I just don't want to have the idea that we are trying to do something that matches OE here. This is outside of our normal 4GL compatibility mindset since there is no equivalent in OE.

Most 4GL applications implement their own application-level login prompt to secure access. We are trying to implement the idea that there is only one login and with that we can accomplish both the OS login and the application authentication.

#27 Updated by Galya B about 1 year ago

What I was actually trying to understand was if the customer wants to get rid of something they actually have in their own setup, which seems to be the case. Although in a completely different implementation. The step doesn't exist in both FWD converted code and the customer's OE code, but it exists in both FWD and customer's setup. Interesting.

#28 Updated by Galya B about 1 year ago

Also I wanted to confirm that from authentication point the OE code doesn't need to be hidden behind OS user. Web exposes the app to the whole internet and if the app doesn't have proper authentication that might be an issue.

#29 Updated by Greg Shah about 1 year ago

Today, we do indeed do the OS authentication first. If that fails, then the user is not allowed to spawn the FWD client. Thus they would never get to the 4GL appl-level login.

In the future solution, we would still have this 2 phase process "under the covers" but there would be only 1 prompt to the user.

#30 Updated by Galya B about 1 year ago

I may be wrong in many aspects. Just sharing my thought process.

Considering all tasks I see these requirements for web login to OE GUI apps without explicit OS login form:
  • OS account with OS permissions set explicitly by the admin in the deployment env. It's needed for the OE app to operate properly on the OS resources.
  • OS account mapped to an end-user account (one to many) explicitly by the admin. FWD has no way to predict what user should have what OS rights and it's currently done exactly in that way, when the admin sets up the remote access. It could be one default OS account for all end-users, but it will have all rights needed for execution of the whole app, so I guess it's not recommended. If multiple OS accounts have to be supported, then the admin needs to import end-user account names from AD to FWD and (see #3931#note-31) set the mapping manually. Then OS users will be like OS permission groups.
  • On initial page load the OS account can't be determined, so the separate spawner process doesn't contribute anything. It can access the native login methods from the Java code. Before running the converted app, there should be this new FWD service that redirects to a supported AD, determines the OS user based on the entered end-user account and spawns the customer app passing in the auth details. Let's not get too specific in this task, the main point is that the OS user needs to be determined after the end user is entered / authenticated.
  • Web sessions to be created for a combination of browser identifier + end-user instead of OS users. Here comes the licensing policies before starting the client process and also mapping a web session to port.
  • FWD security context to be created for a combination of browser identifier + end-user instead of OS users.

I might be straying in a wrong direction, so all of it is under discussion. It just makes sense to me.

#31 Updated by Galya B about 1 year ago

Actually the mapping of end-users and OS accounts is best to be done in the AD and the AD can return an additional field that specifies it, so that complexity will be taken off FWD. Obviously the initial proposal is not a good one.

#32 Updated by Galya B about 1 year ago

P.S. AD groups themselves can have the names of the OS accounts set in the env.

#33 Updated by Greg Shah about 1 year ago

Please do not use references to AD. We are not encoding a solution that knows about Active Directory.

#34 Updated by Galya B about 1 year ago

Greg Shah wrote:

Please do not use references to AD. We are not encoding a solution that knows about Active Directory.

I was considering #4571, but since this is not going to be a default, then please tell how to map end-user to OS account.

#35 Updated by Galya B about 1 year ago

Maybe then the original suggestion applies. In any case the admin should single-handedly map users to OS accounts. It can be a huge config file or in the Admin UI or coming from identity provider.

#36 Updated by Greg Shah about 1 year ago

Most of our customers use Linux for deployment. None of them use Active Directory for management of Linux accounts. Some may use Active Directory for their Windows desktops. Some may have other solutions for Linux or no centralized authentication at all. It varies. Thus we cannot assume AD is part of the solution.

I was considering #4571, but since this is not going to be a default, then please tell how to map end-user to OS account.

We will encode that knowledge in the directory.

#37 Updated by Greg Shah about 1 year ago

OS account with OS permissions set explicitly by the admin in the deployment env. It's needed for the OE app to operate properly on the OS resources.

This is correct, if by "set", you mean that these OS accounts are created in the operating system using tools that are external to FWD.

OS account mapped to an end-user account (one to many) explicitly by the admin.

Yes. It may be "one to one" or "one to many".

FWD has no way to predict what user should have what OS rights

Yes. These are set in the OS as needed.

and it's currently done exactly in that way, when the admin sets up the remote access.

I'm not sure what you mean by remote access in this context. It is true that the admin must configure the permissions for the user at the OS account level. Whether the login is local or remote does not matter.

It could be one default OS account for all end-users, but it will have all rights needed for execution of the whole app, so I guess it's not recommended.

Yes

If multiple OS accounts have to be supported, then the admin needs to import end-user account names from AD to FWD and (see #3931#note-31) set the mapping manually.

We don't need any import capability. The admin will have to define these things in the directory. Currently, that will be a manual editing process.

Then OS users will be like OS permission groups.

I'm not sure what you mean by this.

On initial page load the OS account can't be determined,

Yes

so the separate spawner process doesn't contribute anything.

We will still have a spawner process happening, it just would happen in a different order.

It can access the native login methods from the Java code.

Yes, there would be some "plugin" or hook that is invoked. In #3770 and #4571 there are discussions of what this might look like. Whatever we do here, it needs to be flexible enough for us to implement multiple different possible backend authentication options.

Before running the converted app, there should be this new FWD service that redirects to a supported AD, determines the OS user based on the entered end-user account and spawns the customer app passing in the auth details. Let's not get too specific in this task, the main point is that the OS user needs to be determined after the end user is entered / authenticated.

Roughly, yes, except AD is not part of the solution.

Web sessions to be created for a combination of browser identifier + end-user instead of OS users. Here comes the licensing policies before starting the client process and also mapping a web session to port.

I'm not sure I would say it this way, because the web session (even today) is not related to the OS account. The OS user is still critical for spawning the FWD client, just as it is today. However, we will be trying to map specific browsers + users to specific ports as well as being able to store associated state in the browser's offline storage.

FWD security context to be created for a combination of browser identifier + end-user instead of OS users.

As noted above, the OS user isn't related to the security context today. In the future version, it will be linked only from the perspective that it will be calculated from the user identity.

#38 Updated by Galya B 11 months ago

Is mapping end-users to OS accounts in a manual way even possible? I guess the customer systems support sign-up and deleting / disabling users, or the amount of users that log to their systems is not that substantial?

#39 Updated by Galya B 11 months ago

The question about the amount of end-users is also related to the OS user pooling implementation. If the pooling can't provide enough OS users for all end-users, then it's meaningless. And if the required number is high, the admins won't set it up properly.

#40 Updated by Galya B 11 months ago

Also is it an option to consider the possible implications of running multiple end-users with the same OS user or just presume the unknowns are heavier? We can reason about it based on the implemented Progress features that involve the OS system interactions.

#41 Updated by Galya B 11 months ago

I'm not sure if I got this right: Is the spawner running as admin to allow spawning client processes with any other user? If that's so, do we run the server with the same elevated permissions as the spawner or that's not necessary? (The question might not be related. I'm just trying to comprehend the whole process.)

#42 Updated by Galya B 11 months ago

Another one to verify I got right: Are temp users used only to authenticate NativeSecureConnection before the server, when creating a session?

#43 Updated by Greg Shah 11 months ago

Is mapping end-users to OS accounts in a manual way even possible? I guess the customer systems support sign-up and deleting / disabling users, or the amount of users that log to their systems is not that substantial?

Most customer deployments of the web clients are "branch offices" that have between 5 and 25 users. We do have some customer deployments that expect hundreds of users.

Regardless of the number of users, there really is no alternative. We must have an OS account in which we can spawn the JVM client. In single sign-on mode, we are eliminating the ability for the user to provide this account to us at login, so we must map it. That means an admin must setup these mappings in our configuration.

Optimally, we will provide an admin console screen to define these mappings. Then the mappings can be maintained at runtime without bringing the server down.

The question about the amount of end-users is also related to the OS user pooling implementation. If the pooling can't provide enough OS users for all end-users, then it's meaningless. And if the required number is high, the admins won't set it up properly.

This isn't optional. There must be a properly maintained mapping otherwise single signon will not work.

Also is it an option to consider the possible implications of running multiple end-users with the same OS user

Yes. This is what I describe in #4129-5.

I'm not sure if I got this right: Is the spawner running as admin to allow spawning client processes with any other user?

Yes, it is required to be "setuid" enabled (in Linux terms) in order to launch the FWD client session in an account that is different from the server's OS account.

If that's so, do we run the server with the same elevated permissions as the spawner or that's not necessary?

No, we strongly recommend running the server in a normal user account that only has the minimum privileges to access its jars/libs, config, logs and any critical server-side resources. That limits the potential damage if the server process is ever breached.

In addition, this FWD server OS account absolutely MUST be a different OS account from the FWD clients otherwise those clients might have access to security sensitive server resources (e.g. the directory.xml).

Are temp users used only to authenticate NativeSecureConnection before the server, when creating a session?

Yes.

#44 Updated by Galya B 11 months ago

If the login screen is 4GL app controlled by the customer, how do we manage different sessions? We'll need a dedicated auth server. Now we just serve a static page and then redirect to the full blown 4GL app living in it's own client process.

#45 Updated by Galya B 11 months ago

4GL app needs OS user. OS user is determined after app login. App login needs 4GL app. And we have an infinite loop.

#46 Updated by Greg Shah 11 months ago

If the login screen is 4GL app controlled by the customer, how do we manage different sessions? We'll need a dedicated auth server.

Yes, some service will be needed. It doesn't have to be a separate process but it might be. That is up to the customer and it will potentially be different for each one.

We have to design the hook/callback and how it must work. The customer will implement that hook, possibly with a separate auth server.

Now we just serve a static page and then redirect to the full blown 4GL app living in it's own client process.

In the future, we would be able to configure this auth hook as an alternative. If not there, we would still default to the current approach.

4GL app needs OS user. OS user is determined after app login. App login needs 4GL app. And we have an infinite loop.

This hook will not be 4GL code.

#47 Updated by Galya B 11 months ago

Greg Shah wrote:

This hook will not be 4GL code.

I mean their login page being 4GL will have to live a complete Conversion/client JVM life.

#48 Updated by Greg Shah 11 months ago

As part of moving to this approach, they will modify the application to bypass their normal 4GL login. We will only ever spawn a client for an authenticated user so they don't need to implement the authentication in the 4GL anymore.

#49 Updated by Galya B 11 months ago

Greg Shah wrote:

For #3770-81, the simplest way is to provide a directory-configured auth hook which checks the credentials using some custom 4GL-like Java code (i.e. check some table for the user/password pair).

Agreed. I would add these thoughts:

  • We should allow for this to be "real" converted 4GL code, not just hand written Java code. I think most customers will find it easiest to extract their existing authentication code and put it into a 4GL procedure.

As part of moving to this approach, they will modify the application to bypass their normal 4GL login.

I'm probably missing something here. If we allow the customer to own the login page and write it in 4GL, that will be on itself a full blown 4GL app that needs its own lifecycle before spawning the base / main app.

I have other ideas in mind, but I want to understand this one.

#50 Updated by Greg Shah 11 months ago

I agree that the new login code is NOT 4GL code. That is what I meant in #3931-46 when I said "This hook will not be 4GL code.". There is no infinite loop here nor is there a need for a mini-4GL app. There is no 4GL code involved.

#51 Updated by Greg Shah 11 months ago

We would implement the login outselves and just call the hook which is non-interactive code implemented by the customer to our API specification.

#52 Updated by Galya B 11 months ago

Sorry for asking multiple times. Just want to make sure the old comments are not related, since it's quite important. Thanks for the clarification.

We would implement the login outselves and just call the hook which is non-interactive code implemented by the customer to our API specification.

This sounds good.

#53 Updated by Galya B 11 months ago

Continue from #7465.

Greg Shah wrote:

We cannot require OAuth2 as part of this interface. We are providing support for more than just <org>. We have other customers with completely bespoke authentication processes. For example, some will have their own database tables for authentication.

All customers requesting sso login will have to provide authentication server - 3rd party integration that we support or their own implementation in a separate service. In case of their own implementation being the choice, we have to have a contract with that service.

With sso the app can either rely purely on FWD and don't do any authentication, or we need to use a standard interface that will work with 3rd party auth providers, so that FWD can forward to the 4GL apps the result of the authentication in an expected format, so that they can manage the session.

OAuth is supported by 3rd party auth providers including Azure AD, Google and the mentioned KeyCloak. If the customer has highly opinionated login process then we'll have to implement highly opinionated solution. But for the standard implementation, OAuth will cover the most. With just a change of a few directory configs we should be able to switch to another provider.

Also OAuth2 doesn't mean there is no control over the db tables. The server implementation is in the hands of the service developer. node-oauth2-server js lib for example has a model contract and it's up to the dev to wire it up. Even the response to the standard auth request can return custom fields.

Anyways, I think we'll need OAuth2 if we want to cover 3rd party providers with the least amount of efforts.

#54 Updated by Greg Shah 11 months ago

All customers requesting sso login will have to provide authentication server - 3rd party integration that we support or their own implementation in a separate service. In case of their own implementation being the choice, we have to have a contract with that service.

I don't see this as a separate service. The API should be called in-process in the FWD server. This API will have its class registered in the directory. The implementation of the API is up to the customer. We don't care what it does.

It may call a 3rd party auth server. Or it may read something from the application database. Or something else. As long as it honors the contract of the API, we don't care.

Although our API should be usable with OAuth2, there is no reason that we should implement based on that solution alone.

#55 Updated by Galya B 11 months ago

Greg Shah wrote:

I don't see this as a separate service.

I think we speak about different things here. It's not FWD that needs a separate service, but the customer. 4GL hasn't been started, we need to make login request. The problem is that previously they had full control over the login process, while now their login screen should be removed, so first we need to make a valid auth request and then we need feed the 4GL app the result of it.

The actual auth provider connection can be outside of FWD indeed. In that separate service - customer's code that does DB querying or auth provider request.

#56 Updated by Galya B 11 months ago

The most simple flow would be:
1. SSO Login screen is open
2. User/pass entered
3. FWD hits the customer's auth server and gets response success, the fwd user and some session specific data, blob
4. FWD spawns 4GL app and exposes the blob to 4GL so that it can manage the session on its own any way it likes

#57 Updated by Greg Shah 11 months ago

FWD hits the customer's auth server and gets response success, the fwd user and some session specific data, blob

This is where I disagree. A separate auth server is not required or desired. The API should be implemented in-process in the FWD server. That enables a range of common use cases which otherwise would be much more difficult if we require a separate server.

Implementing the hook in-process, does not stop a customer from implementing a separate auth server. They can do so, but it will be invoked from the in-process API implementation rather than directly from FWD.

#58 Updated by Galya B 11 months ago

The auth logic, where DB queries are done, should be part of FWD server?

#59 Updated by Galya B 11 months ago

How are hooks provided? I don't think we have dependency injection or event system built-in.

#60 Updated by Greg Shah 11 months ago

The auth logic, where DB queries are done, should be part of FWD server?

It would be Java code that is written by the customer and run inside of the FWD server. That way it has full access to the converted database, which will be a common requirement.

#61 Updated by Greg Shah 11 months ago

How are hooks provided? I don't think we have dependency injection or event system built-in.

A class implementing the proper interface can be defined in the directory. We do this in many cases already. For example, see the session and server init/term hooks.

#62 Updated by Galya B 11 months ago

I want to create 4GL attr AUTH-BLOB, do we add it to SESSION as well?

#63 Updated by Greg Shah 11 months ago

Yes, if we go that route. Let's get everything described and agreed first.

#64 Updated by Galya B 10 months ago

The auth-plugin has to be changed for web, because that's where the FWD user is associated with the security context and there is no applicable auth-plugin now that can call the expected new auth hook. But I'm not sure if changing the auth-plugin to a new that serves the SSO flow will affect other clients that are expected to run with GuestAccess for example.

The thing is web will have a login page to get the fwd user (mapped to os user) and that will be made available to the new auth-plugin to enable spawning the client under the right user. But standalone gui/batch for example won't have that login screen and can't use the new plugin.

I'm not sure if mixed clients should be / could be possible.

#65 Updated by Galya B 10 months ago

Basically sessions will have to be created under the fwd user associated with the login form request.

Some identifier will have to be passed to the spawner -> to the client -> back to the server to keep the reference, just to keep trustedspawner an option. Otherwise the client can be spawned directly from the server with a native call.

At the moment Virtual Desktop doesn't use trustedspawner and os auth requires user and pass, while embedded uses only trusted. I guess we want to make Virtual Desktop allow association of fwd users to os users without passwords with trusted spawner? Having both options for both web drivers?

Side note: Trusted spawner may be a convenience for admins, but complicates the flow a lot.

#66 Updated by Galya B 10 months ago

  • Status changed from New to WIP
  • Assignee set to Galya B

Branch 3931a created.

#67 Updated by Greg Shah 10 months ago

But I'm not sure if changing the auth-plugin to a new that serves the SSO flow will affect other clients that are expected to run with GuestAccess for example.

The GuestAccess plugin is just for use in two cases:

  • If the application handles all authorization in the 4GL code, then the GuestAccess plugin can allow the FWD security model to be "bypassed" by forcing a standard account to be automatically assigned, leaving all security in the hands of the application.
  • There might be some cases where a web site or kiosk (using a non-web UI) might want to startup with a fixed account for some "public" usage.

It is not expected to be used for the SSO case.

The thing is web will have a login page to get the fwd user (mapped to os user) and that will be made available to the new auth-plugin to enable spawning the client under the right user. But standalone gui/batch for example won't have that login screen and can't use the new plugin.

Actually, I think the non-web clients should get a login dialog when the SSO plugin is active. There is no reason this SSO capability is specific to web only.

I'm not sure if mixed clients should be / could be possible.

Yes, they should be.

#68 Updated by Greg Shah 10 months ago

I guess we want to make Virtual Desktop allow association of fwd users to os users without passwords with trusted spawner? Having both options for both web drivers?

Yes.

#69 Updated by Galya B 10 months ago

Sandbox in 3931a based on trunk 14637, commits 14638 & 14639.

What I did:
  • The same login page and POST request handling in WebHandler are used, but under a new path /sso/gui. This new path triggers sso app user / pass validation through a simple SSO auth interface SsoAuthenticator.
  • The implementation of SsoAuthenticator is provided in directory with a new config <node-attribute name="ssoplugin" value="com.goldencode.p2j.security.SsoAuthenticatorSample"/> under config/auth-mode/. The sample impl simply returns success and bogus. A real implementation can also hardcode the fwd user in some or all responses.
  • The new method SecurityManager.getOsUserCred maps the fwd user received from SsoAuthenticator to an os user:
    • If fwd user is null, it fallbacks to "default" leaving an option open for admins who don't want to work and don't want to hardcode it in the sso authenticator.
    • If the fwd user is not found in SecurityCache, the login exists with a new error code / message.
    • Fwd user, e.g. accounts/users/bogus, can have attribute ospool / osuser, both or none.
    • When fwd user has attribute osuser, directory looks for the os user is accounts/osusers/OSUSERNAME and checks the node's class is osuser.
    • When fwd user has no attribute osuser, but attribute ospool, directory looks for the pool accounts/osusers/OSPOOLNAME and checks the node's class is ospool. Then gets the next os user from the pool (strategy to be implemented, currently hard-coded).
    • When fwd user has no attribute osuser or ospool, directory looks for the pool accounts/osusers/default. Then gets the next os user from the pool (strategy to be implemented, currently hard-coded).
  • On successful sso authentication, the app user / pass are substituted in spawnWorker with the mapped os user / pass and the flow continues as it is originally when os credentials arrive from the login screen directly.
  • osuser can have no pass attribute, then trusted/passwordless is automatically enabled.

Working configs:

      <node class="container" name="config">
        <node class="authMode" name="auth-mode">
          <node-attribute name="mode" value="4"/>
          <node-attribute name="retries" value="-1"/>
          <node-attribute name="plugin" value="com.goldencode.p2j.security.GuestAccess"/>
          <node-attribute name="ssoplugin" value="com.goldencode.p2j.security.SsoAuthenticatorSample"/>
        </node>
      </node>
      <node class="container" name="accounts">
        <node class="container" name="osusers">
          <node class="ospool" name="trusted">
            <node-attribute name="passwordless" value="gbb,osuser2"/>
          </node>
          <node class="ospool" name="default">
            <node class="osuser" name="osbogus">
              <node-attribute name="pass" value="pass"/>
            </node>
            <node class="osuser" name="osbogus1">
              <node-attribute name="pass" value="pass1"/>
            </node>
            <node class="osuser" name="osbogus2">
              <node-attribute name="pass" value="pass2"/>
            </node>
          </node>
          <node class="osuser" name="osuser23">
            <node-attribute name="pass" value="pass23"/>
          </node>
          <node class="osuser" name="gbb"/>
        </node>
        <node class="container" name="users">
          <node class="user" name="bogus">
            <node-attribute name="enabled" value="TRUE"/>
            <node-attribute name="person" value="A B. C"/>
            <node-attribute name="alias" value="shared"/>
            <node-attribute name="protected" value="FALSE"/>
            <node-attribute name="groups" value="everybody"/>
            <node-attribute name="ospool" value="trusted"/>
            <node-attribute name="osuser" value="gbb"/>
          </node>
        </node>
      </node>

The result of the efforts is: If you change "gbb" in the example configs with your os username and navigate to https://localhost:7443/sso/gui, enter any credentials, the client process will get spawned. Basically what has been achieved is to have a call to an external sso auth implementation that provides fwd user, mapped in directory to os user or os pool. The spawner now works with the os credentials provided in directory and verifies them. When the os user has no password, the code automatically switches to passwordless / trusted.

TODO (a lot):
  • move directory parsing from SecurityManager to SecurityCache;
  • figure out how to feed the fwd user associated with the login to the connection authenticator (GuestAccess or an alternative);
  • pass encryption in directory;
  • admin UI interface;
  • ...

Disclaimer: This is just a POC.

#70 Updated by Galya B 10 months ago

I'm still not sure if we need to enable SSO server-wide or just for certain clients. When SSO will be server-wide always (I mean no exception cases are foreseen) when the config is enabled and that is confirmed, I can then remove the new path /sso.

#71 Updated by Galya B 10 months ago

If auto-login is supported and the cookie is not deleted on session completion, the redirect back to the login page will lead to recursion.

#72 Updated by Galya B 10 months ago

Also it's so convenient, I can already imagine the constant lack of ports if no restrictions are imposed.

#73 Updated by Galya B 10 months ago

The auth code in SecurityManager (authenticateClientWorker / authenticateLocal) is very special and I don't feel qualified to modify it to accommodate for SSO.

I need help.

I don't know how to feed the fwd user from WebHandler spawnWorker (contained in SsoAuthenticator.Result) to SecurityManager.

#74 Updated by Galya B 10 months ago

Embedded doesn't have a default landing page, while VD has. For embedded index.html where <iframe id="embeddedP2J"> is defined is supplied directly by the customer app.

To enable browser fingerprint sent on sign-in and stable origin for browser configs, registry and auto-login, we need control over the page.

But there is a lot going on in the embedded index.html outside the login form. Do we have customers using embedded at the moment? I want to know it's safe to rework it.

#75 Updated by Galya B 10 months ago

H * E * L * P

I don't understand something basic. Why does it matter what security context spawns the web client? The client will later create a new session with the server and that's where the actual context should matter.

So EmbeddedWebAppHandler.AuthWorker doesn't make sense to me. It creates a thread and its own context with SecurityManager.authenticateServer just to execute the same Webhandler.spawnWorker as VD, where no explicit context is created.

#76 Updated by Galya B 10 months ago

I'm not sure if I've messed up something or this is how embedded is supposed to work, but when I define a startup procedure with some basic operations that exits right after that, embedded is not closing / redirecting back to the login page. While VD does exit. Is this how embedded is supposed to be?

#77 Updated by Galya B 10 months ago

Is it... is it... the remote spawn that needs proper security context?

#78 Updated by Greg Shah 10 months ago

Is it... is it... the remote spawn that needs proper security context?

I don't understand the question. Security context is needed for many things.

From email:

Is the security context created for embedded to do remote spawn (connect to brokers)?

Embedded mode should be independent of whether spawning is done through a broker or not. I don't know if any specific linkage there.

What specific security context are you referring to here? The user's session itself has a security context, like all usage of converted code would require. You're probably not referring to that.

#79 Updated by Galya B 10 months ago

I'm trying to understand why virtual desktop runs without security context and bypasses security checks, while embedded needs it.

#80 Updated by Galya B 10 months ago

According to the comments in the code it seems related to trusted mode, but it still doesn't make sense.

#81 Updated by Galya B 10 months ago

It is like the server says: Let me check my configs. ... a lot going on ... I'm all good. Now do the same as VD that doesn't need any configs.

#82 Updated by Galya B 10 months ago

VD has this line sp.bypass = !SecurityManager.getInstance().hasContext();.

#83 Updated by Greg Shah 10 months ago

I'm not sure if I've messed up something or this is how embedded is supposed to work, but when I define a startup procedure with some basic operations that exits right after that, embedded is not closing / redirecting back to the login page. While VD does exit. Is this how embedded is supposed to be?

Embedded mode is based on the idea that there is a web application which will be a hand-written, custom Javascript application using any kind of web frameworks or web development approaches. Inside that application, we embed an iframe. Inside that iframe we load a modified version of our web client and connect it to FWD via the same websocket approach used normally by the virtual desktop mode. At this point we are not expecting a normal "startup program" to execute. Instead, we expect that the web application will use a Javascript API (which we've provided) to handle navigational control for the iframe. To start with, this means we "up call" to Java to get a menu tree which we can render in any way needed. When the user makes some decision to navigate, we use that Javascript API to load the screen requested into the iframe. Selecting a different choice will switch the screens. We have ways of listing the screens, switching between active screens, opening new screens, closing screens... a navigational API. The Javascript API also provides mechanisms to "up call" to invoke arbitrary logic in the converted code (similar to how appserver works) and we have integration facilities for 4GL publish/subscribe so that code in the custom web app can publish or subscribe to 4GL named events just like any other converted 4GL code. This gives a clean way to implement integration without tightly coupling the code (e.g. it avoids the more complicated appserver model of tight coupling).

Given the above, it is not normal to configure embedded mode the same way (e.g. a converted 4GL startup program) as you would expect with virtual desktop mode. Hotel GUI is a good working example.

I should also note that we treat windows differently in embedded mode. We don't render them with any of the window decorations since we aren't simulating the "virtual desktop". There are other differences too, especially in relation to modal windows (dialogs) which must run synchronously above the existing screen in the iframe, controlling the user input and then when dismissed returning back to the existing screen. The web app does not manage that process, it is handled by the legacy converted code.

#84 Updated by Greg Shah 10 months ago

I'm trying to understand why virtual desktop runs without security context and bypasses security checks, while embedded needs it.

The virtual desktop definitely has a security context associated with the session. Perhaps you are talking about how we use the temporary accounts to establish the new session?

VD has this line sp.bypass = !SecurityManager.getInstance().hasContext();.

Where?

#85 Updated by Galya B 10 months ago

I'm speaking only about the spawning process and the Thread with the security context doesn't do anything to my knowledge except spawning the same way (with the same method as VD).

#86 Updated by Galya B 10 months ago

Greg Shah wrote:

I'm trying to understand why virtual desktop runs without security context and bypasses security checks, while embedded needs it.

The virtual desktop definitely has a security context associated with the session. Perhaps you are talking about how we use the temporary accounts to establish the new session?

I'm speaking about spawning, no sessions. The WebHandler for embedded creates a bottleneck spawning Thread with security context that has no value in my opinion.

VD has this line sp.bypass = !SecurityManager.getInstance().hasContext();.

Where?

WebHandler.doPost, line 677. It's always true as far as I can test it.

#87 Updated by Galya B 10 months ago

I see EmbeddedWebAppHandler is created by Constantin and he can explain us better why AuthWorker exists.

#88 Updated by Greg Shah 10 months ago

Embedded doesn't have a default landing page, while VD has. For embedded index.html where <iframe id="embeddedP2J"> is defined is supplied directly by the customer app.

Correct.

To enable browser fingerprint sent on sign-in and stable origin for browser configs, registry and auto-login, we need control over the page.

The customer will control it because it is part of their custom web application. We may define reasonable requirements to which that code must adhere. As much as possible, we try to hide such things behind a clean and consistent Javascript API and all the other processing inside the iframe itself, which is completely under our control.

But there is a lot going on in the embedded index.html outside the login form. Do we have customers using embedded at the moment? I want to know it's safe to rework it.

We have multiple customers (at least 3 plus 2 more soon) who use it in some form but it is not in production usage. It can be reworked if we are reasonable about the requirements.

#89 Updated by Constantin Asofiei 10 months ago

Galya B wrote:

I see EmbeddedWebAppHandler is created by Constantin and he can explain us better why AuthWorker exists.

Look at WebHandler.spawn method on line 250 - this is used only for embedded mode. Without a FWD context, the TrustedSpawnerResource checks could not be done.

#90 Updated by Galya B 10 months ago

Constantin Asofiei wrote:

Galya B wrote:

I see EmbeddedWebAppHandler is created by Constantin and he can explain us better why AuthWorker exists.

Look at WebHandler.spawn method on line 250 - this is used only for embedded mode. Without a FWD context, the TrustedSpawnerResource checks could not be done.

There is the extra layer I don't understand, but eventually both do the same WebHandler.spawnWorker (line 365).
Why do we need checks for TrustedSpawnerResource ? It's very imaginary to me, we create an imaginary resource and create a requirement to auth before it and then in the same place (directory) set the key to unlock the achievement.

#91 Updated by Galya B 10 months ago

I mean obviously spawner doesn't require specific auth to call the process and it works with passwordless. If the server authenticated before itself, what is the achievement?

#92 Updated by Constantin Asofiei 10 months ago

Galya B wrote:

Why do we need checks for TrustedSpawnerResource ? It's very imaginary to me, we create an imaginary resource and create a requirement to auth before it and then in the same place (directory) set the key to unlock the achievement.

This is for security concerns - in trusted mode (no password is specified for the OS user under which the FWD client is spawned), we need to know a list of OS users which are allowed; otherwise, any OS user can be used to launch the FWD client.

#93 Updated by Galya B 10 months ago

VD works with OS user / pass, they auth in PAM -> pass.
Embedded works with certificates associated with alias associated with FWD user, that needs to be verified if it can spawn under the hardcoded os user in TrustedSpawnerResource. The certificate in this flow don't seem very straight forward match, but maybe that's what we got for other purposes, why not use it here as well. So in embedded what we verify is if the fwd user is listed under TrustedSpawnerResource as far as I understand and just by chance the fwd user comes from the certificate that is used somewhere else as well?

#94 Updated by Galya B 10 months ago

It's not nit picking, I really need to get to the bottom of it to be able to rework those to accommodate for SSO and more in general (like implementing the TODOs).

So the problem I have with this is there is no validation of any user input, there is nothing dynamic in the equation, no remote connection that needs to be authenticated, it's all on the server. The server is validating the server for configurations. If embedded is enabled, just let it be: allow it to always spawn under the provided fwd user that is configured for the certificate. Maybe we need to check if the fwd user is configured before that, but it's not a bad idea for VD as well. Am I missing something?

#95 Updated by Greg Shah 10 months ago

Some context and decision criteria:

  • The reason this task is entitled "single sign-on for virtual desktop mode" is that we already have a hack for embedded mode that does something similar. As noted above, the intention has always been to implement a solution that can work for both virtual desktop mode and embedded mode.
  • The reference to "single sign-on" is actually NOT a reference to the traditional concept of SSO which is to implement a single login to a trusted 3rd party auth server and then implement authorization based on some returned token. The reference is more related to the elimination of the extra OS login needed for spawning web clients. This means:
    • We are implementing hooks that allow customers to plug in their own authentication processing/code to our process.
    • Although we would like to maintain a true SSO concept, it is not required and it is not the core idea here. We do have customers with interest in the idea (e.g. #4571) but we also have customers that have no interest. Rather than encoding a more complicated external mapping of our process to OAuth2 or other SSO mechanisms, we are instead focused on enabling the capability (and more) to be plugged in.
  • We have an existing authentication hook in FWD, which is the Authenticator interface. This is a very similar concept with these differences:
    • It is called within the session creation process and is tightly coupled to the SM.authenticateLocal() (server side for interactive non-web clients) and SM.authenticateClientWorker() (client side for interactive non-web clients). We can ignore SM.authenticateSingle(), it is probably going to be removed at some point, it is not used.
    • This tight integration and synchronous design does not work for the web client cases.
    • The mechanism is server driven but actually allows the delivery of a custom login UI as a Java class that is run on the client but lives on the server. We serialize the bytecode and load it on the client side so that the client doesn't have to have the application jar loaded. That UI is customer written and was meant to be able to be compatible with the converted code. It is responsible for prompting the user for credentials (userid/password).
    • The authentication itself is done in FWD, using the FWD security model. This means it is completely based on the FWD accounts and the actual authentication is not implemented by the customer.
    • Think of this authentication hook as being more of a UI exit point rather than the authentication logic itself being hooked.
    • Two customers use this in production today, both are ChUI. We haven't tried implementing this in GUI though I think it would work.
    • Technically, it could be used with web clients as well but in our existing web client approach we already determine the FWD account before that point and the FWD session creation itself is non-interactive.

The primary focus of the hook/API being created here is certainly for web clients. I do wonder if the interactive non-web clients should be considered as well.

I'm still not sure if we need to enable SSO server-wide or just for certain clients. When SSO will be server-wide always (I mean no exception cases are foreseen) when the config is enabled and that is confirmed, I can then remove the new path /sso.

I think when this is enabled it affects all web clients. I guess that is what you mean by "server-wide always".

#96 Updated by Greg Shah 10 months ago

TrustedSpawnerResource is not specific to embedded mode. It could be used in virtual desktop mode to avoid the PAM authentication. We don't want to hard code dependencies (in embedded or virtual desktop mode) related to how the spawning works.

The fact that our current embedded mode cases use TrustedSpawnerResource is just because we were trying to achieve something that avoided the OS login. Ultimately, we want the same approach to be possible for both web client modes.

#97 Updated by Greg Shah 10 months ago

The auth code in SecurityManager (authenticateClientWorker / authenticateLocal) is very special and I don't feel qualified to modify it to accommodate for SSO.

Yes, this is a complicated part of the problem to solve. I suspect we need to add another AUTH_MODE_* and associated logic to bypass our own authentication.

I don't know how to feed the fwd user from WebHandler spawnWorker (contained in SsoAuthenticator.Result) to SecurityManager.

I think we already have something like this for use of the TemporaryAccount approach, though I dislike that current mechanism. Optimally I would want a one-time use cert/token approach where the spawned client rendezvous with an account that was just authenticated.

#98 Updated by Galya B 10 months ago

Very good explanation, will be helpful for everyone jumping in later on. Unfortunately I already got it figured out by now the hard way.

It could be used in virtual desktop mode to avoid the PAM authentication.

Not at the moment. That's what I'm trying to achieve here understanding the current setup.

#99 Updated by Galya B 10 months ago

Greg Shah wrote:

I suspect we need to add another AUTH_MODE_* and associated logic to bypass our own authentication.

I think when this is enabled it affects all web clients. I guess that is what you mean by "server-wide always".

auth-mode is defined centrally for the whole server and affects all clients, it's usually 4 with enabled plugin GuestAccess. If I create a new mode or a new custom auth plugin, it will automatically apply to other types of clients as well, if the directory configs are not reworked to somehow specify which type of client works with which type of auth. Then also the code in SecurityManager should be modified to know which type of client it is that gets authenticated.

#100 Updated by Galya B 10 months ago

Enabling trusted for VD (which is currently not possible) should probably be done the same way as embedded. But the code in embedded doesn't make much sense, so I can't work on it, before it's explained.

I don't understand what does the certificate have to do with the spawning process in embedded. Is it only to identify the fwd user and check if it's allowed to spawn clients? But why not, we configure the certificate and its alias, the fwd user and the permissions for spawning all in the same directory.xml. Also why do we need a context to run the spawning for embedded instead of just checking the validity of the configs if that is at all needed.

#101 Updated by Galya B 10 months ago

Directory configs under server/default/embeddedWebApp are all used only in EmbeddedWebAppHandler.AuthWorker. That is authKeyStore, authKeyStorePassword, authAlias. The only work the AuthWorker does (outside validating these configs) is WebHandler.spawnWorker, the same as VD mode (in @WebHandler.doPost). I don't know if I'm missing something here. Maybe there was another idea originally and that's how it ended being.

#102 Updated by Galya B 10 months ago

Greg Shah wrote:

I don't know how to feed the fwd user from WebHandler spawnWorker (contained in SsoAuthenticator.Result) to SecurityManager.

I think we already have something like this for use of the TemporaryAccount approach

You're right. This is awesome. There is basically a mechanism to feed the user / pass to the session from the client. I'll see where I can get from there.

#103 Updated by Constantin Asofiei 10 months ago

Galya B wrote:

Directory configs under server/default/embeddedWebApp are all used only in EmbeddedWebAppHandler.AuthWorker. That is authKeyStore, authKeyStorePassword, authAlias. The only work the AuthWorker does (outside validating these configs)

The validation is done by TrustedSpawnerResource, which requires for the thread to have a FWD context - we don't want a FWD server thread doing this, that's why a thread with its own context will do this.

is WebHandler.spawnWorker, the same as VD mode (in @WebHandler.doPost).

Yes, in the end it ends up in this code, common with the Virtual Desktop client.

#104 Updated by Galya B 10 months ago

Constantin Asofiei wrote:

Galya B wrote:

Directory configs under server/default/embeddedWebApp are all used only in EmbeddedWebAppHandler.AuthWorker. That is authKeyStore, authKeyStorePassword, authAlias. The only work the AuthWorker does (outside validating these configs)

The validation is done by TrustedSpawnerResource, which requires for the thread to have a FWD context - we don't want a FWD server thread doing this, that's why a thread with its own context will do this.

But TrustedSpawnerResource is just a resource definition. Where in the code is the check if the thread has context?

#105 Updated by Constantin Asofiei 10 months ago

Galya B wrote:

Constantin Asofiei wrote:

Galya B wrote:

Directory configs under server/default/embeddedWebApp are all used only in EmbeddedWebAppHandler.AuthWorker. That is authKeyStore, authKeyStorePassword, authAlias. The only work the AuthWorker does (outside validating these configs)

The validation is done by TrustedSpawnerResource, which requires for the thread to have a FWD context - we don't want a FWD server thread doing this, that's why a thread with its own context will do this.

But TrustedSpawnerResource is just a resource definition. Where in the code is the check if the thread has context?

Look at tsta.isAllowed(user) calls - it ends up calling SecurityContextStack.getContext() via getCachedDecision, which needs a context.

#106 Updated by Galya B 10 months ago

Ok, let's get it straight: EmbeddedWebHandler checks if webClient/defaultAccount is the same as trustedspawner/resource-instance/reference and that's why a Thread with security context is needed. Also the check is done once when creating the Thread, but then all requests to /embedded go through the same Thread. May I just replace that whole logic with if (Utils.getNodeWhatever("webClient/defaultAccount").equals(Utils.getNodeWhatever("trustedspawner/resource-instance/reference"))?

#107 Updated by Galya B 10 months ago

That was pseudo-code of course, but my point is that I still don't see a good reason to keep the context.

#108 Updated by Galya B 10 months ago

Also:
1. Why is there a certificate specified for embedded? I still don't get that as well. Is it for the same check?
2. Also security permissions can be changed in admin UI if I'm not mistaken, so this check needs to be done every time for each request which won't be cheap.

#109 Updated by Constantin Asofiei 10 months ago

Galya, the point of:

   public static String spawn(String   user,
                              String   pw,
                              boolean  gui,
                              String[] options,
                              String   referrer,
                              String[] requestParameters)
   {


was to allow to receive a specific user from the web request. Currently the call is this:
               url[0] = WebHandler.spawn(null,
                                         null,
                                         true,
                                         options,
                                         "https://www.google.com",
                                         WebHandler.getRequestParameters(request));

Currently, the user argument is null, so it uses the webClient/defaultAccount. This in turn must match the trustedspawner/resource-instance/reference value.

This is just the 'default way of working', and this wasn't enhanced to allow the embedded web app to send its own OS user. As I understand, this still needs to be possible with #4129.

May I just replace that whole logic with if (Utils.getNodeWhatever("webClient/defaultAccount").equals(Utils.getNodeWhatever("trustedspawner/resource-instance/reference"))?

You can't replace it like that. The trustedspawner/resource-instance/reference is an ACL - which can be accessed only via a FWD context.

1. Why is there a certificate specified for embedded? I still don't get that as well. Is it for the same check?

The certificate is associated with a FWD process account - this will be used to authenticate and establish the FWD context.

2. Also security permissions can be changed in admin UI

Exactly, we can't cache ACL decisions (outside the SecurityContext.decisionCache).

As long as the security check for the OS user passed to WebHandler.spawn is done via ACLs, the FWD context is required.

#110 Updated by Galya B 10 months ago

Constantin Asofiei wrote:

1. Why is there a certificate specified for embedded? I still don't get that as well. Is it for the same check?

The certificate is associated with a FWD process account - this will be used to authenticate and establish the FWD context.

Only for this exact context that is need just to check trustedspawner/resource-instance/reference against another value?

#111 Updated by Galya B 10 months ago

Ok, here goes my proposal for lean code:
1. Expose a method from SecurityManager that checks trustedspawner/resource-instance/reference against a value.
2. Remove the thread and the context from embedded web handler and the certificate.

#112 Updated by Constantin Asofiei 10 months ago

No, this is not the single reason. I forgot to mention that the embedded app (for i.e. Hotel GUI) can be ran in standalone mode. This app will:
  • launch its own Web server
  • connect to the FWD server and authenticate via a certificate for a FWD process account (thus will have a FWD context)
  • call into the FWD server via WebClientLauncher.launch, where it can specify any OS user (although currently is hard-coded to null).

So, at least with current approach, the FWD context is needed.

1. Expose a method from SecurityManager that checks trustedspawner/resource-instance/reference against a value.

This does not work without a FWD context - the ACLs are specific to each account

#113 Updated by Galya B 10 months ago

Constantin Asofiei wrote:

  • call into the FWD server via WebClientLauncher.launch, where it can specify any OS user (although currently is hard-coded to null).

These "static" interfaces that are not implemented syntactically always get me.

One missing implements and you need to spend 5h to explain to me why the code looks overcooked.

#114 Updated by Galya B 10 months ago

Constantin Asofiei wrote:

  • connect to the FWD server and authenticate via a certificate for a FWD process account (thus will have a FWD context)

Maybe a bit more clarification on that front. The standalone web server will have the certificate to create a standard session with the FWD server. Does it have anything to do with EmbeddedWebAppHandler.AuthWorker? Because the connection auth of rpc calls is on a low level, while this web handler obviously runs the context in a new unrelated Thread and to me it doesn't seem related. In FWD's code the certificate is used only for the Thread in EmbeddedWebAppHandler.AuthWorker.

#115 Updated by Galya B 10 months ago

certificates/peers and private-keys are probably needed for the server <-> server connection auth for rpc calls, but the embeddedWebApp/authKeyStore and the related Thread and its context is just to check os user and fwd user are allowd by trustedspawner. Am I correct?

#116 Updated by Galya B 10 months ago

Also another question: the spawn calls to the brokers run at the moment in different contexts. Embedded has this certificate driven context, while VD has the default server context. How are brokers configured? Do we have brokers for VD and embedded tested?

#117 Updated by Galya B 10 months ago

Early review required on 3931a r14652 based on trunk r14637.

What was done:
  • adding /sso/gui, /sso/chui and /sso/embedded context paths for SSO login;
  • adding SSO API / hook: SsoAuthenticator with sample implementation (src/com/goldencode/p2j/security/SsoAuthenticatorSample.java); Modified GuestAccess, SecurityManager.authenticateLocal and SecurityManager.sendAuthType to make connection authentication sso aware.
  • supporting ospool and osuser directory configs (src/dir_schema.xml); fwd user mapping to osuser/ospool (SecurityManager.getOsUserCred);
  • unifying the spawn logic for VD and Embedded web handlers (VirtualDesktopWebHandler and EmbeddedWebHandler extending WebHandler);
  • providing thread pool for security context for security checks for embedded trusted (EmbeddedWebHandler.securityContextExecutorService);
  • providing alternative security context for VD passwordless / trusted and PAM / non-trusted (no longer bypassing security checks) and embedded PAM / SSO (SecurityManager.authenticateWebWorker);
  • supporting auto-login for VD SSO (still needs a logout screen, maybe that should be the flag to enable it).

Directory configs working with SsoAuthenticatorSample.java:

      <node class="container" name="accounts">
        <node class="container" name="osusers">
          <node class="ospool" name="trusted">
            <node-attribute name="passwordless" value="gbb,osuser2"/>
          </node>
          <node class="ospool" name="default">
            <node class="osuser" name="osbogus">
              <node-attribute name="pass" value="pass"/>
            </node>
            <node class="osuser" name="osbogus1">
              <node-attribute name="pass" value="pass1"/>
            </node>
          </node>
          <node class="osuser" name="osuser23">
            <node-attribute name="pass" value="pass23"/>
          </node>
          <node class="osuser" name="[OSUSER]"/>
        </node>
        <node class="container" name="users">
          <node class="user" name="newuser">
            <node-attribute name="enabled" value="TRUE"/>
            <node-attribute name="person" value="A B. C"/>
            <node-attribute name="alias" value="shared"/>
            <node-attribute name="protected" value="FALSE"/>
            <node-attribute name="groups" value="everybody"/>
            <node-attribute name="osuser" value="[OSUSER]"/>
          </node>
        </node>
      </node>
      <node class="container" name="config">
        <node class="authMode" name="auth-mode">
          <node-attribute name="mode" value="4"/>
          <node-attribute name="retries" value="-1"/>
          <node-attribute name="plugin" value="com.goldencode.p2j.security.GuestAccess"/>
          <node-attribute name="ssoplugin" value="com.goldencode.p2j.security.SsoAuthenticatorSample"/>
        </node>
        <node class="container" name="auth-plugins">
          <node class="container" name="guest_login">
            <node class="string" name="classname">
              <node-attribute name="value" value="com.goldencode.p2j.security.GuestAccess"/>
            </node>
            <node class="string" name="description">
              <node-attribute name="value" value="Non-Interactive Guest Auto-Login"/>
            </node>
            <node class="string" name="option">
              <node-attribute name="value" value="bogus"/>
            </node>
          </node>
        </node>
      </node>

Replace [OSUSER] with a valid OS username. When pass is configured, it will be used for PAM authentication, when it's missing, trusted is enabled.
And add newuser to all security/acl/net where bogus is listed.

There is a lot of incomplete work and documentation, but I need an early review to confirm the approach and discuss possible holes in it.

#118 Updated by Galya B 10 months ago

Do we have customers using brokers with web (VD / embedded)?

#119 Updated by Greg Shah 10 months ago

Do we have customers using brokers with web (VD / embedded)?

Not in production today but it should be supported.

#120 Updated by Greg Shah 10 months ago

I don't have time to do a full code review.

Some quick feedback:

  • As noted in #4129, the pooling side of things is not critical right now. What is most important is being able to map a specific FWD account to a specific OS account. We have customers planning a 1-for-1 approach and also a "all users are the same OS account" approach. At this time we don't have "I want a pool of OS users that get randomly assigned". For that reason, I'd prefer to keep the solution simpler in regard to pooling.
  • The authentication mode will be universal for all web clients so I expect that the sso/ parts of the URL will go away.
  • We have one customer that plans a separate web server for their embedded environment and all the other current customers that plan to use embedded mode will use the current "integrated" Jetty server in the FWD server. I don't want to disallow this separate server concept BUT I also don't want the common case to be much more complicated because of that separate server case. The current work to create a pool of threads to handle the trusted spawning is that more complicated case.
    • I would like to consider changes to how we host embedded mode for that separate server case if we can simplify things.
    • The trusted spawner itself is not the most important thing here. The behavior is what is important. It seems to me that relying upon the existing trusted spawner is not the approach I would prefer.
  • I would expect us to drop usage of GuestAccess in the solution. It is a hack that is not important/is unwanted when one implements SSO.

In regard to unifying the spawning logic and the authentication changes, I'll have to just look at the code before I can offer any feedback.

Constantin: Please review.

#121 Updated by Galya B 10 months ago

Greg Shah wrote:

  • As noted in #4129, the pooling side of things is not critical right now. [...] For that reason, I'd prefer to keep the solution simpler in regard to pooling.

I intended to close #4129 with 3931a. Can we fulfill the requirements of #4129 without support for a pool?

#122 Updated by Galya B 10 months ago

Greg Shah wrote:

  • We have one customer that plans a separate web server for their embedded environment and all the other current customers that plan to use embedded mode will use the current "integrated" Jetty server in the FWD server. I don't want to disallow this separate server concept BUT I also don't want the common case to be much more complicated because of that separate server case. The current work to create a pool of threads to handle the trusted spawning is that more complicated case.

/embedded/launch request without SSO runs only in trusted mode, because the server has only a default os account and no password for it. It currently uses one Thread only (in AuthWorker) to queue all spawning work for the sake of security checks. Implementing Thread pool was in TODO and I implemented it already, there is nothing complicated in it. The rpc interface will have its own security context and should not be needing Thread pool.

#123 Updated by Galya B 10 months ago

That being said, I'm still not sure what security context spawning should be executed under to be able to authenticate before brokers, so this logic may change, but the current limitation of the default embedded setup should still be taken into consideration.

#124 Updated by Galya B 10 months ago

Greg Shah wrote:

  • I would expect us to drop usage of GuestAccess in the solution. It is a hack that is not important/is unwanted when one implements SSO.

The connection authentication flow starts with the client asking for session and sending the AUTH_REQ type (user, program, process). At this point the server knows nothing else and checks its server-wide auth-mode/mode and auth-mode/plugin to use the right Authenticator. There a few keypoints here.

I don't understand why the flow is so complicated, why do we have so many steps of authentication and so much back and forth. What I did is just to plug in the fwd user associated with sso into the GuestAccess flow. I don't see a point to replicate the same behavior in a separate mode + Authenticator. Now on spawn the servers sends the fwd user to the client and stores clientUuid : fwd user in a map. On creating the session, the client sends the fwd user and the uuid to the server and the server verifies these match, then the flow continues as it does usually with GuestAccess.

Is there any reason why this is not a good solution?

#125 Updated by Galya B 10 months ago

Greg Shah wrote:

  • I would expect us to drop usage of GuestAccess in the solution. It is a hack that is not important/is unwanted when one implements SSO.

As far as I understand it's a valid case to have other clients running without SSO on the same server, so I don't think we can just drop it.

#126 Updated by Galya B 10 months ago

Greg Shah wrote:

In regard to unifying the spawning logic and the authentication changes, I'll have to just look at the code before I can offer any feedback.

I have to apologize beforehand, the diff doesn't work any more since the refactoring was major. I simplified tons of code, added new and merged some methods. It is fairly easy to follow the new code, but diff is not an option.

#127 Updated by Greg Shah 10 months ago

  • As noted in #4129, the pooling side of things is not critical right now. [...] For that reason, I'd prefer to keep the solution simpler in regard to pooling.

I intended to close #4129 with 3931a. Can we fulfill the requirements of #4129 without support for a pool?

Yes. As I noted in #4129-26, our current requirements don't require pooling. I'm OK with avoiding that complexity for now.

Your dream of closing #4129 with 3931a remains possible. :)

#128 Updated by Greg Shah 10 months ago

That being said, I'm still not sure what security context spawning should be executed under to be able to authenticate before brokers, so this logic may change, but the current limitation of the default embedded setup should still be taken into consideration.

My point about avoiding the thread pool and contexts for processing authentication is for this reason:

  • Yes, thread pools are not hard to implement but they add cost (memory, CPU, extra code to maintain and debug) and complexity that is best avoided whenever possible.
  • The previous trusted spawner approach was done that way because of our initial separate server approach to embedded. I'd like to get rid of the separate server approach completely and simplify the authentication process as a result.

This is just a statement of what I would like to achieve. It remains to be determined if what I want is achievable.

#129 Updated by Galya B 10 months ago

Actually for rpc calls to spawn thread pool is not used, because security context should be available. The Thread pool is for http requests for launching the client, because these threads are created by Jetty and have no context. After the fwd user is determined, the current Thread needs to get associated with a security context to do the security checks for the fwd user.

#130 Updated by Greg Shah 10 months ago

Is there any reason why this is not a good solution?

We'll see. I will have to look at the code. It sounds like you are mixing two different authenticator implementations together which may cause confusion. In a perfect world we would have a single authenticator concept that could be used for both.

I wouldn't have expected GuestAccess to be the core approach. The class itself is even named to suggest this.

#131 Updated by Galya B 10 months ago

Greg Shah wrote:

It sounds like you are mixing two different authenticator implementations together which may cause confusion.

SsoAuthenticator is an API, it's not a connection authenticator. Our current hooks are quite a different story, very simplified in that they know - it's a server, pass it that user, it's a client, pass something else. But sso authentication should be done once in the web page flow and does have nothing to do with rpc connections. Also it can't replace all client (non-web including) auth at once (server-wide config).

#132 Updated by Greg Shah 10 months ago

Galya B wrote:

Actually for rpc calls to spawn thread pool is not used, because security context should be available. The Thread pool is for http requests for launching the client, because these threads are created by Jetty and have no context. After the fwd user is determined, the current Thread needs to get associated with a security context to do the security checks for the fwd user.

Right. So it is cleaner if we don't have an existing context and instead we should take the passed in application credentials and use them to authenticate with the customer's registered authenticator. If and only if that succeeds, we now would have the FWD account that should be our context AND we have been told by this registered authenticator implementation that this account is already authenticated. So we can now create a context for that account. I don't see why we need an existing security context to do that, it is an unnecessary complication that is only there because of our historical "separate server" approach.

#133 Updated by Galya B 10 months ago

Greg Shah wrote:

So it is cleaner if we don't have an existing context and instead we should take the passed in application credentials and use them to authenticate with the customer's registered authenticator. If and only if that succeeds, we now would have the FWD account that should be our context AND we have been told by this registered authenticator implementation that this account is already authenticated. So we can now create a context for that account.

That's exactly what I've implemented.

I don't see why we need an existing security context to do that, it is an unnecessary complication that is only there because of our historical "separate server" approach.

That is only to support the old embedded web requests. Are we moving all customers using embedded to SSO? If not, that was just a TODO left in the code, a bottleneck that is fixed.

#134 Updated by Galya B 10 months ago

To summarize embedded spawning / launching options:
1. [Old] web request to /embedded/launch runs without knowing anything, that's why uses a webClient/defaultAccount and certificate associated with fwd account to do the security checks by processing all requests in a single Thread (now Thread pool)
2. [Old] rpc call to spawn running in its own security context; doesn't need anything explicitly
3. [New] SSO web request that knows the fwd user and takes advantage by setting up the security context on the current thread

#135 Updated by Greg Shah 10 months ago

  • I would expect us to drop usage of GuestAccess in the solution. It is a hack that is not important/is unwanted when one implements SSO.

As far as I understand it's a valid case to have other clients running without SSO on the same server, so I don't think we can just drop it.

I don't think it makes sense to simultaneously have more than one way to authenticate web client sessions. I'm not saying GuestAccess is removed. I'm saying it wouldn't be used in the same server at the same time as SSO. In #3931-120: I said:

The authentication mode will be universal for all web clients so I expect that the sso/ parts of the URL will go away.

I still agree with this. The URL should not be the control point for how a user gets authenticated. When active SSO will be in force.

#136 Updated by Galya B 10 months ago

So batch processes will run on that server with SSO? auth-config/plugin defines GuestAccess used by batch. Do I remove it, when web uses SSO?

#137 Updated by Greg Shah 10 months ago

Are we moving all customers using embedded to SSO?

Yes. We already have a hacked-up way to do this. With #3770 and #3931 we intend to implement a much cleaner/architected approach.

In other words, our existing hack for embedded mode already provides a kind of SSO. I want to move to a better approach rather than extend the hack.

#138 Updated by Greg Shah 10 months ago

Galya B wrote:

So batch processes will run on that server with SSO? auth-config/plugin defines GuestAccess used by batch. Do I remove it, when web uses SSO?

My references above are only discussing web clients. Of course, batch and non-web clients should not be broken. But the SSO doesn't apply to them so why would they be modified?

If needed we can change configuration and/or how the existing non-web sessions work. That is OK as long as we can easily migrate everyone. But conceptually, we should not remove or break the existing support.

#139 Updated by Galya B 10 months ago

Greg Shah wrote:

Are we moving all customers using embedded to SSO?

Yes.

Let me verify I understand correctly. Current customers using embedded will remove their login screens and will implement their Java SSO authenticator on moving to our latest version?

#140 Updated by Greg Shah 10 months ago

Are we moving all customers using embedded to SSO?

Yes.

Let me verify I understand correctly. Current customers using embedded will remove their login screens and will implement their Java SSO authenticator on moving to our latest version?

Yes. We will migrate existing code (that already does the same thing using a more complicated approach).

#141 Updated by Galya B 10 months ago

Greg Shah wrote:

My references above are only discussing web clients. Of course, batch and non-web clients should not be broken. But the SSO doesn't apply to them so why would they be modified?

Ok, auth-mode will be utterly disregarded by web then (not that it's the first client doing it) and web client processes will inform the server what they want - SSO authentication, which will then happen in one step. Let me see if I can do it in 5 lines of code.

#142 Updated by Galya B 10 months ago

r14653: GuestAccess reverted. Sso connection authentication is SecurityManager.authenticateClientWorkerSso client-side and SecurityManager.authenticateLocalSso server-side. The only variables in that authentication are client uuid and fwd user, if both match the server creates a context and the client has its AuthData.

#143 Updated by Galya B 10 months ago

At the moment, as far as I understand the logic, authentication is either certificate check, or fwd users check. If net:connection:secure is true, then the alias of X509Certificate is used to create the context. But isn't it proper to have both certificate validation and user validation?

#144 Updated by Galya B 10 months ago

What I mean is having certificate should not limit the number of associated fwd users. Isn't it good to have clients with certificates spawned by SSO with 1:1 fwd users. I'm actually curious why certificates are not set to all clients always.

#145 Updated by Greg Shah 10 months ago

At the moment, as far as I understand the logic, authentication is either certificate check, or fwd users check.

I don't think this is correct.

Cert check is AUTH_MODE_X509, userid/password check is AUTH_MODE_IDPW and if both are required then it is AUTH_MODE_X509_IDPW.

The first two are certainly the most used authmodes today. The 3rd is a "belt and suspenders" option that gives a higher level of security.

The 4th option is AUTH_MODE_CUSTOM which not only allows the customer to conrol the login dialog but it also can provide the authentication on the back end. As such, it certainly should be considered a separate mode than the other 3.

If net:connection:secure is true, then the alias of X509Certificate is used to create the context. But isn't it proper to have both certificate validation and user validation?

I don't understand this. There are times when AUTH_MODE_X509_IDPW is wanted but we can't always assume it is appropriate. Requiring a certificate is a big administrative burden and we can't force customers down that path.

#146 Updated by Galya B 10 months ago

I didn't notice AUTH_MODE_X509_IDPW, AUTH_MODE_X509_IDPW does indeed both validations, but the FWD user is either single one coming from configs, or received interactively that doesn't work for processes.

My question is if it should be an option to enable certificate validation + SSO?

#147 Updated by Greg Shah 10 months ago

My question is if it should be an option to enable certificate validation + SSO?

Help me understand how would it work. For example, where does the cert reside? Is it in the browser's certificate storage and then has to be passed to the server and made to work with the Java cert code?

#148 Updated by Galya B 10 months ago

I mean the connection between the FWD client process (with embedded web server) and the FWD server.

#149 Updated by Galya B 10 months ago

With SSO it can't be of type AUTH_MODE_X509_IDPW (except if we don't modify it), because the fwd user is dynamic (can't be read from configs or client interface in the interactive way).

#150 Updated by Greg Shah 10 months ago

It would be feasible to implement certificate validation + SSO with the customer hook returning the certificate to use to establish the session. I don't want to eliminate that possibility but we don't have a customer planning to use it right now.

#151 Updated by Galya B 10 months ago

Just to mention, so that no unexpected surprises are found down the road: "sso hook" is a bit exaggerated for what I've implemented in r14653 (#3931-142) and I think it's a very good solution. When someone can review the few lines of code, I can elaborate more on the details.

#152 Updated by Galya B 10 months ago

Brokers are currently using only certificate authentication before the server and the session is running for the whole lifespan of the broker. SSO requires different sessions for different logins, which should mean multiple different sessions on the broker at the same time created on demand on spawn. Is broker support for SSO going to be a separate task?

#153 Updated by Galya B 10 months ago

Actually that is not correct. The broker is used just for spawning a separate process, so the connection to the server is irrelevant to the context needed for execution of the client. This just means that the server thread calling the broker should have the same certificate context expected by the broker. Embedded trusted mode (from http request to /launch) at the moment does have a context derived by certificate. But VD SSO and no SSO don't have the proper context, also rpc calls to WebClientLauncher.spawn are probably running in a context based on what the client requests and may not have proper context.

I think I've asked about it once already, but let's make it clear if this task should enable broker use for any of the above.

#154 Updated by Greg Shah 10 months ago

Yes, brokers need to be supported. This is just a spawning mechanism it should not really be core to the concept of SSO.

As far as RPC calls go, the broker model should work off of a queue, not a direct RPC call. Any authorized context should be able to add a spawn request to the queue.

#155 Updated by Greg Shah 10 months ago

By "any authorized context", I mean a context and caller that is allowed to spawn a new session.

#156 Updated by Galya B 10 months ago

The current web flow is:
WebHandler.spawnWorker -> (Web)ClientSpawner.spawn -> [broker for the os user is found] WebClientBuilder.remoteStart -> ClientBuilder.remoteStart -> BrokerManager.spawn directly calls broker.getRemote().spawn(args) / BrokerClientServices.spawn.

AppserverLauncher -> Scheduler -> ProcessLaunchTask.run -> ProcessClientSpawner.launch -> ClientSpawner.spawn -> (Process)ClientBuilder.remoteStart -> same [...]

On launch, the broker starts a session, authenticated by the certificate associated with a process on the server, and then calls BrokerManager.registerBroker. The server saves the remote object of the BrokerClientServices network interface. So it seems this trick is what I couldn't understand. In this case the calling Thread doesn't need proper authentication, it uses the stored object with its session.

#157 Updated by Galya B 10 months ago

Now a question about

allowed to spawn a new session

The BrokerServerServices acl/net config described here seems outdated. Does it need to be removed from the document?

Also is it replaced by remotelaunchoption?

And remotelaunchoption checks seem to be used only in the web client spawning process. Is it how it's supposed to be? Also does remote launch mean broker or the checks should be done for any type of web client launch?

#158 Updated by Constantin Asofiei 9 months ago

Review for 3931a rev 14653:
  • ClientDriver.main - defaultUncaughtExceptionHandler can be null
  • SchemaComparatorTest, SchemaValidatorTest - what were the reason to change from org.junit to org.junit.jupiter.api?
  • 'static final' constants like SchemaLoad.required we want them in uppercase
  • no actual change as of rev 14653 - Authenticator.java, CustomHookSample.java, DatabaseAuthenticationHook.java, DefaultLoginPanel.java, GuestAccess.java, TrustedClientPlugin.java
  • SpawnErrorCodes.errorMessage - for case 1 and 2 (invalid user/password), if the error message is reported to the web browser or otherwise available in a response, it must be Invalid username or password. The same for the error code. We can't allow info about valid or invalid users to be exposed to the client. I think this is an existing issue.
  • SecurityManager.authenticateLocalSso
    • authenticateWebWorker's 'checkCallerAbort' needs this new caller
    • I don't think we can ignore issues when writing on the socket, this would mean a failure; just let the IOException be thrown and caught by the caller.
    • why is the socket.addSessionListener(null); call?
  • WebHandler.processSsoResult - why do we default to fwdUser = "default" if the ssoAuthResult has no FWD user specified? We can't hard-code this, at most we can read a configuration from the directory
  • there are new .html files for SSO mode - is there anything to take care in terms of #7067 (same for the refactoring related to VirtualDesktopWebHandler.java and EmbeddedWebHandler.java)?
  • SecurityContextThreadFactory - change the name of the Java thread to state that this is for the embedded web handler, SecurityContextExecutorThread is too generic.
  • when SsoAuthenticator.authenticate is called, what context does the thread have? Does it have the FWD Server context? I ask because we may want to run this login code in a AssociatedThread with the context of some FWD process (maybe reusing SecurityContextThreadFactory pool).

#159 Updated by Galya B 9 months ago

Constantin Asofiei wrote:

Review for 3931a rev 14653:
  • ClientDriver.main - defaultUncaughtExceptionHandler can be null

If the exception is not thrown again, the process doesn't exit like it will with any unhandled exception, so I reused the default handler as well.

  • SchemaComparatorTest, SchemaValidatorTest - what were the reason to change from org.junit to org.junit.jupiter.api?

These few test classes were not compiling due to the change of the classpath, where jupiter is available, but not the old junit. I guess noone has noticed, because noone runs these tests. I tried to add new tests and the cmd failed with compilation errors. On checking the classpath, it indeed doesn't include the old api. I mean someone can have a custom cmd to run the tests fine at the moment, but the one in gradle doesn't work for the old junit api.

  • 'static final' constants like SchemaLoad.required we want them in uppercase

I can't find a change introduced by me here except for reusing the var, but I can fix it.

  • no actual change as of rev 14653 - Authenticator.java, CustomHookSample.java, DatabaseAuthenticationHook.java, DefaultLoginPanel.java, GuestAccess.java, TrustedClientPlugin.java

Yes, changed were reverted. I'll revert the empty chars as well.

  • SpawnErrorCodes.errorMessage - for case 1 and 2 (invalid user/password), if the error message is reported to the web browser or otherwise available in a response, it must be Invalid username or password. The same for the error code. We can't allow info about valid or invalid users to be exposed to the client. I think this is an existing issue.

Error messages were copied from the old WebHandler to SpawnErrorCode. Old msgs are not changes, I only added the last few cases. But I'll fix it.

  • SecurityManager.authenticateLocalSso
    • authenticateWebWorker's 'checkCallerAbort' needs this new caller

It's a private method, does it need checkCallerAbort?

  • I don't think we can ignore issues when writing on the socket, this would mean a failure; just let the IOException be thrown and caught by the caller.

In authenticateLocal same exceptions are some ignored, some logged and ignored, none are thrown for further handling. I've followed the flow. It's way too complicated and low level for me to introduce a change.

  • why is the socket.addSessionListener(null); call?

It simulates line 3223 in the same revision. Otherwise an exception is thrown, because there is logic related to the null value passed in like creating an empty collection or something.

  • WebHandler.processSsoResult - why do we default to fwdUser = "default" if the ssoAuthResult has no FWD user specified? We can't hard-code this, at most we can read a configuration from the directory

We can, but my understanding is that the devs already had their chance to return a default based on a login in the SsoAuthenticator. Are they going to be more capable working with configs? Actually we may need to even remove that "default". A configurable fwd user for sso in directory or a default in the code would provoke laziness and the expected 1:1 mapping (app user : fwd user) will not be fulfilled.

  • there are new .html files for SSO mode - is there anything to take care in terms of #7067 (same for the refactoring related to VirtualDesktopWebHandler.java and EmbeddedWebHandler.java)?

I originally thought embedded may require changes to the login page, but it actually doesn't, so I'll introduce the concept of global server config for SSO and remove the context path /sso and also serve the same file. As for #7067 changes are still not introduced.

  • SecurityContextThreadFactory - change the name of the Java thread to state that this is for the embedded web handler, SecurityContextExecutorThread is too generic.

It's a private class and it won't be used outside EmbeddedWebHandler, so the name needs to bring context only for that class.

  • when SsoAuthenticator.authenticate is called, what context does the thread have? Does it have the FWD Server context? I ask because we may want to run this login code in a AssociatedThread with the context of some FWD process (maybe reusing SecurityContextThreadFactory pool).

WebClientLauncher.spawn remote calls come in with FWD context and I still haven't considered well this case. Does it need sso and fwd user context? The other spawn requests come from web and are running on Jetty threads without fwd context. When sso is disabled for embedded, the SecurityContextThreadFactory is used by passing in the executor to WebHandler.spawnWorker. For sso, since the FWD user is dynamic (it's coming from the SsoAuthenticator result) thread authentication will be required every time before the security checks, in spawnWorker it does if (executor == null && !SecurityManager.getInstance().hasContext()). The remote spawn itself doesn't seem to need a context, because it's a saved instance of the network interface of the broker, received on registration.

#160 Updated by Constantin Asofiei 9 months ago

Galya B wrote:

Constantin Asofiei wrote:

Review for 3931a rev 14653:
  • ClientDriver.main - defaultUncaughtExceptionHandler can be null

If the exception is not thrown again, the process doesn't exit like it will with any unhandled exception, so I reused the default handler as well.

I don't understand; from looking at the code, the default value for Thread.defaultUncaughtExceptionHandler is null - are you saying that this is being set by the JVM to some other value?

  • SecurityManager.authenticateLocalSso
    • authenticateWebWorker's 'checkCallerAbort' needs this new caller

It's a private method, does it need checkCallerAbort?

Yes. Otherwise the call will not be allowed.

  • I don't think we can ignore issues when writing on the socket, this would mean a failure; just let the IOException be thrown and caught by the caller.

In authenticateLocal same exceptions are some ignored, some logged and ignored, none are thrown for further handling. I've followed the flow. It's way too complicated and low level for me to introduce a change.

I meant in authenticateLocalSso - let the exceptions in that code be thrown.

  • why is the socket.addSessionListener(null); call?

It simulates line 3223 in the same revision. Otherwise an exception is thrown, because there is logic related to the null value passed in like creating an empty collection or something.

OK. Please fix NetSocketBase.getSessionListeners to return an empty list instead of null. And you can remove socket.addSessionListener(null);.

  • WebHandler.processSsoResult - why do we default to fwdUser = "default" if the ssoAuthResult has no FWD user specified? We can't hard-code this, at most we can read a configuration from the directory

We can, but my understanding is that the devs already had their chance to return a default based on a login in the SsoAuthenticator. Are they going to be more capable working with configs? Actually we may need to even remove that "default". A configurable fwd user for sso in directory or a default in the code would provoke laziness and the expected 1:1 mapping (app user : fwd user) will not be fulfilled.

Lets consider this a misconfiguration and if the SSO plugin didn't specify a FWD user, then return an error code.

  • SecurityContextThreadFactory - change the name of the Java thread to state that this is for the embedded web handler, SecurityContextExecutorThread is too generic.

It's a private class and it won't be used outside EmbeddedWebHandler, so the name needs to bring context only for that class.

The name needs to bring context to a thread dump; or while debugging and looking at the thread list. Please make it more verbose.

  • when SsoAuthenticator.authenticate is called, what context does the thread have? Does it have the FWD Server context? I ask because we may want to run this login code in a AssociatedThread with the context of some FWD process (maybe reusing SecurityContextThreadFactory pool).

WebClientLauncher.spawn remote calls come in with FWD context and I still haven't considered well this case. Does it need sso and fwd user context? The other spawn requests come from web and are running on Jetty threads without fwd context. When sso is disabled for embedded, the SecurityContextThreadFactory is used by passing in the executor to WebHandler.spawnWorker. For sso, since the FWD user is dynamic (it's coming from the SsoAuthenticator result) thread authentication will be required every time before the security checks, in spawnWorker it does if (executor == null && !SecurityManager.getInstance().hasContext()). The remote spawn itself doesn't seem to need a context, because it's a saved instance of the network interface of the broker, received on registration.

What I mean is for non-embedded clients: a customer will write a plugin (which can be any kind of code, even 4GL-converted code), and this will be executed via SsoAuthenticator.authenticate. This code needs a proper security context, always.

#161 Updated by Galya B 9 months ago

Constantin Asofiei wrote:

I don't understand; from looking at the code, the default value for Thread.defaultUncaughtExceptionHandler is null - are you saying that this is being set by the JVM to some other value?

I've never seen null default exception handler, it sounds right that it's set by the JVM.

  • SecurityManager.authenticateLocalSso
    • authenticateWebWorker's 'checkCallerAbort' needs this new caller

It's a private method, does it need checkCallerAbort?

Yes. Otherwise the call will not be allowed.

Sorry. I was looking at the latest revision. checkCallerAbort has been removed, even the method is renamed. Consider checked.

  • I don't think we can ignore issues when writing on the socket, this would mean a failure; just let the IOException be thrown and caught by the caller.

In authenticateLocal same exceptions are some ignored, some logged and ignored, none are thrown for further handling. I've followed the flow. It's way too complicated and low level for me to introduce a change.

I meant in authenticateLocalSso - let the exceptions in that code be thrown.

There are some flaws actually. I'll fix the logic around the failures, but the exception handling still can be done in the method, otherwise more logic will be added to the 700 lines authenticateLocal method and it will be all the same: log the exception and return null. I'll notify when done.

OK. Please fix NetSocketBase.getSessionListeners to return an empty list instead of null. And you can remove socket.addSessionListener(null);.

Ok.

  • WebHandler.processSsoResult - why do we default to fwdUser = "default" if the ssoAuthResult has no FWD user specified? We can't hard-code this, at most we can read a configuration from the directory

We can, but my understanding is that the devs already had their chance to return a default based on a login in the SsoAuthenticator. Are they going to be more capable working with configs? Actually we may need to even remove that "default". A configurable fwd user for sso in directory or a default in the code would provoke laziness and the expected 1:1 mapping (app user : fwd user) will not be fulfilled.

Lets consider this a misconfiguration and if the SSO plugin didn't specify a FWD user, then return an error code.

Sounds good.

  • SecurityContextThreadFactory - change the name of the Java thread to state that this is for the embedded web handler, SecurityContextExecutorThread is too generic.

It's a private class and it won't be used outside EmbeddedWebHandler, so the name needs to bring context only for that class.

The name needs to bring context to a thread dump; or while debugging and looking at the thread list. Please make it more verbose.

Oh, you wrote about the thread names, I thought the class name should be changed. Ok, so for thread name we can have EmbeddedContextThread-1.

  • when SsoAuthenticator.authenticate is called, what context does the thread have? Does it have the FWD Server context? I ask because we may want to run this login code in a AssociatedThread with the context of some FWD process (maybe reusing SecurityContextThreadFactory pool).

WebClientLauncher.spawn remote calls come in with FWD context and I still haven't considered well this case. Does it need sso and fwd user context? The other spawn requests come from web and are running on Jetty threads without fwd context. When sso is disabled for embedded, the SecurityContextThreadFactory is used by passing in the executor to WebHandler.spawnWorker. For sso, since the FWD user is dynamic (it's coming from the SsoAuthenticator result) thread authentication will be required every time before the security checks, in spawnWorker it does if (executor == null && !SecurityManager.getInstance().hasContext()). The remote spawn itself doesn't seem to need a context, because it's a saved instance of the network interface of the broker, received on registration.

What I mean is for non-embedded clients: a customer will write a plugin (which can be any kind of code, even 4GL-converted code), and this will be executed via SsoAuthenticator.authenticate. This code needs a proper security context, always.

It's decided that 4GL won't work for SSO auth, because: 1. SSO should happen before any 4GL process is spawned to determine the os user 4GL spawns in and 2. having 4GL for login will require separate authentication server to run the 4GL plugin. So Greg confirmed it a while ago that the SSO authenticator will be a Java class.

#162 Updated by Galya B 9 months ago

Basically we can't have a context before determining the fwd user that is returned by the SSO auth, so it should run without context. It will probably be a call to 3rd party directory or some customer's authentication server with its own auth method.

#163 Updated by Greg Shah 9 months ago

Correct. This will be pure Java and will not have a context.

#164 Updated by Constantin Asofiei 9 months ago

Thanks for the explanation.

Greg, just an idea: it would be helpful for i.e. development mode to have a 'GuestSSOPlugin' which uses the defaultAccount for OS user and a configurable FWD user (like we have for GuestAccess), to be able to run tests without the OS login page.

#165 Updated by Galya B 9 months ago

I created the SsoAuthenticatorSample and hardcoded fwd user newuser that can use any os user configured for it.

          <node class="user" name="newuser">
            <node-attribute name="enabled" value="TRUE"/>
            <node-attribute name="person" value="A B. C"/>
            <node-attribute name="alias" value="shared"/>
            <node-attribute name="protected" value="FALSE"/>
            <node-attribute name="groups" value="everybody"/>
            <node-attribute name="osuser" value="gbb"/>
          </node>

I wanted it to be different from bogus to actually see what it affects, what permissions are related. So the question is what will be the name of the hardcoded fwd user in SsoAuthenticatorSample that can be linked to any os user in directory.

#166 Updated by Galya B 9 months ago

I mean we can still use bogus. When I'm done with the work, I'll change it to bogus and will add <node-attribute name="osuser" value="[os-user]"/> to its attributes in testcases and other similar projects.

#167 Updated by Constantin Asofiei 9 months ago

Galya B wrote:

I mean we can still use bogus. When I'm done with the work, I'll change it to bogus and will add <node-attribute name="osuser" value="[os-user]"/> to its attributes in testcases and other similar projects.

I'd prefer to read these from the directory.

#168 Updated by Galya B 9 months ago

The fwd user attr <node-attribute name="osuser" value="[os-user]"/> can't be replaced by a default. That's how the mechanism works for sso, based on the mapping. Replacing the default fwd user with config instead of hardcoding it in the code may sound good, but this config will be very peculiar to a test plugin only. I'm not inspired.

#169 Updated by Constantin Asofiei 9 months ago

Check how auth-plugins works with GuestAccess - something similar can be done for the guest SSO plugin (we should rename this to something similar).

#170 Updated by Galya B 9 months ago

I'll soon apply the changes with the latest revision, so when you have time, you can also check the initial refactoring of SecurityManager in r14654. I tried to extract a few classes out of SecurityManager, but then I realized this approach, if followed all through, will require too many changes and too much efforts and cause possible issues that should be tested. So the fallback plan was to keep most of the logic inside the class, but just organize it, so that it can be a good candidate for easy separation later on.

Basically what I achieved is in Eclipse in the Outline view there will be new wrapper classes that help organize important parts of the SecurityManager logic in an self-explanatory way. I have more ideas how to organize the remaining parentless 91 methods, but let's first confirm the approach.

#171 Updated by Galya B 9 months ago

I have a question about the security checks, that are for the plugins trustedspawner and remotelaunchoption: what are these permissions exactly?
  • trustedspawner I guess is allowing the fwd user to spawn a new process without os password.
  • remotelaunchoption I guess is allowing the fwd user to send certain options to the client. Why is it called remote launch? These are exactly the additional options sent via socket to the web client.

At least this is how it's implemented. Is this the actual purpose of their existence?

#172 Updated by Galya B 9 months ago

Since both checks were omitted with VD before, I want to know if it makes sense to enable them for regular VD spawn requests without SSO.

#173 Updated by Galya B 9 months ago

SSO Features Support:

Feature Planned
SSL connection authentication + SSO
OS account pools
RPC WebClientLauncher.spawn with SSO
4GL SSO authentication hook
Default FWD user for SSO

Greg, please review.

Note can be updated at any time.

#174 Updated by Galya B 9 months ago

Actually is it possible to determine the fwd user for Virtual Desktop running without SSO? For the connection auth with GuestAccess the FWD user comes from the auth hook option. Is it possible to use other Authenticator implementations for VD? Is it a solution to add a fwd user getter to the interface Authenticator and only implement it for GuestAccess, so that it's accessible from the web handlers and we can then create security context for normal VD and run the security checks?

#175 Updated by Galya B 9 months ago

r14659 up, all code review requests addressed.

I'll be waiting for answers to my questions and further code review, before proceeding to the finalization steps:
  • encrypted os pass in directory
  • admin ui for os users
  • logout page
  • javadoc, headers
  • testing

#176 Updated by Galya B 9 months ago

Configs for testing:

      <node class="container" name="config">
        <node class="authMode" name="auth-mode">
          <node-attribute name="mode" value="4"/>
          <node-attribute name="retries" value="-1"/>
          <node-attribute name="plugin" value="com.goldencode.p2j.security.GuestAccess"/>
          <node-attribute name="ssoplugin" value="com.goldencode.p2j.security.SsoAuthenticatorSample"/>
          <node-attribute name="ssopluginoption" value="bogus"/>
        </node>
      </node>
      <node class="container" name="accounts">
        <node class="container" name="osusers">
          <node class="osuser" name="[OS-USER]"/>
        </node>
        <node class="container" name="users">
          <node class="user" name="bogus">
            <node-attribute name="enabled" value="TRUE"/>
            <node-attribute name="person" value="A B. C"/>
            <node-attribute name="alias" value="shared"/>
            <node-attribute name="protected" value="FALSE"/>
            <node-attribute name="groups" value="everybody"/>
            <node-attribute name="osuser" value="[OS-USER]"/>
          </node>
        </node>
      </node>

#177 Updated by Greg Shah 9 months ago

trustedspawner I guess is allowing the fwd user to spawn a new process without os password.

Correct

remotelaunchoption I guess is allowing the fwd user to send certain options to the client. Why is it called remote launch? These are exactly the additional options sent via socket to the web client.

I think this was about securing the entry points (e.g. client:cmd-line-option:startup-procedure) that a session can execute based on the FWD account.

Constantin?

#178 Updated by Greg Shah 9 months ago

remotelaunchoption I guess is allowing the fwd user to send certain options to the client. Why is it called remote launch? These are exactly the additional options sent via socket to the web client.

I think this was about securing the entry points (e.g. client:cmd-line-option:startup-procedure) that a session can execute based on the FWD account.

This is primarily useful for non-spawned clients where the inputs are not in our control.

#179 Updated by Greg Shah 9 months ago

Since both checks were omitted with VD before, I want to know if it makes sense to enable them for regular VD spawn requests without SSO.

Neither check is important for virtual desktop mode as it exists today (i.e. unless we expose something new):

  • Forking sessions would need the ability to start clients without the os login but it doesn't need to be implemented using the trustedspawner.
  • If we exposed a facility to allow the user of the web login to enter arbitrary bootstrap config values, then we would need the remotelaunchoption support. This would be quite useful for manual testing environments.

#180 Updated by Constantin Asofiei 9 months ago

Greg Shah wrote:

I think this was about securing the entry points (e.g. client:cmd-line-option:startup-procedure) that a session can execute based on the FWD account.

Not just that. This secures any bootstrap configs which are passed via the spawner (client:cmd-line-option:param, client:web:embedded). Embedded client (standalone app) especially uses this (and this is the 'remote' part, i.e. a 3rd party app drives FWD client spawning).

#181 Updated by Greg Shah 9 months ago

RPC WebClientLauncher.spawn with SSO

This still needs to be discussed. We have a customer that originally planned to have a separate web server with their own web application which would then embed our converted screens. I don't know if that approach is still needed. It was much more complicated than our current built-in approach.

Default FWD user for SSO

Are you asking if we still need to support GuestAccess or some equivalent? The answer there is yes. Also, I agree with Constantin that we must configure this in the directory. It is not reasonable to expect customers or other users of this facility to have to edit FWD to change the account that is used.

#182 Updated by Galya B 9 months ago

Greg Shah wrote:

Default FWD user for SSO

Are you asking if we still need to support GuestAccess or some equivalent? The answer there is yes. Also, I agree with Constantin that we must configure this in the directory. It is not reasonable to expect customers or other users of this facility to have to edit FWD to change the account that is used.

GuestAccess has nothing to do with SSO, not a single bit, so let's not get it involved in the discussion. Both authentications are mutually exclusive, but can both exist on the same server for different processes.

For web once SSO is enabled, it's all in the hands of the customer to provide proper SSO Authenticator, where they can have their default FWD user. All FWD users for SSO are expected to be mapped to a OS user. We can't have a default FWD user, the customer should provide it. Do you want a default os user if none is specified for the FWD user in directory?

For testing, the sample hook can be configured with a default FWD user, but that's a testing authenticator with hardcoded values.

P.S. As I've shown in the test configs above now the sample authenticator can be configured with a default fwd user in directory by ssopluginoption. And again, it doesn't make sense to have default FWD user. You were speaking about 1:1 mapping.

#183 Updated by Galya B 9 months ago

Let me just explain it that way: SSO with a default FWD user and a default OS user is called GuestAccess.

#184 Updated by Constantin Asofiei 9 months ago

Galya B wrote:

Let me just explain it that way: SSO with a default FWD user and a default OS user is called GuestAccess.

And these defaults need to be configured in the directory.

#185 Updated by Galya B 9 months ago

Constantin Asofiei wrote:

Galya B wrote:

Let me just explain it that way: SSO with a default FWD user and a default OS user is called GuestAccess.

And these defaults need to be configured in the directory.

Like with GuestAccess.

#186 Updated by Greg Shah 9 months ago

Do you want a default os user if none is specified in directory?

I would not expect there to be a default os user that is not specified in the directory, but I don't know that my point answers your question.

#187 Updated by Greg Shah 9 months ago

GuestAccess has nothing to do with SSO, not a single bit, so let's not get it involved in the discussion. Both authentications are mutually exclusive, but can both exist on the same server for different processes.

So long as SSO is web-specific, then indeed we can't use it to replace GuestAccess AND we absolutely do need for non-web clients to be supported in the same server as web clients.

#188 Updated by Galya B 9 months ago

Greg Shah wrote:

I would not expect there to be a default os user that is not specified in the directory, but I don't know that my point answers your question.

No. The point in the table was about FWD user. Now I have two questions:
1. Do you want a configurable FWD user for when SSO Authenticator (customer) doesn't know what it wants for security. By not having certain mapping by misconfiguration we'll make the customer comfortable by not letting them know about it and map the failing app user to a default fwd user (something they can do in the authenticator themselves).
2. Do you want a configurable OS user for the FWD user returned by SSO Authenticator when the user is not explicitly mapped to an os user?

#189 Updated by Greg Shah 9 months ago

1. Do you want a configurable FWD user for when SSO Authenticator (customer) doesn't know what it wants for security. By not having certain mapping by misconfiguration we'll make the customer comfortable by not letting them know about it and map the failing app user to a default fwd user (something they can do in the authenticator themselves).

I think the answer here is to provide a facility for the authenticator to get some configuration from the directory. Rather than require each authenticator to implement its own custom directory access, this would probably be best handled by a generic facility. Then this kind of default can be defined in the directory and not hard coded into the authenticator. It also makes a certain class of authenticator more indpendent because they might not have to make changes to a separate authentication server.

2. Do you want a configurable OS user for the FWD user returned by SSO Authenticator when the user is not explicitly mapped to an os user?

Yes, this mapping is something that is more specific to FWD than to the authenticator.

#190 Updated by Galya B 9 months ago

Greg Shah wrote:

I think the answer here is to provide a facility for the authenticator to get some configuration from the directory. Rather than require each authenticator to implement its own custom directory access, this would probably be best handled by a generic facility. Then this kind of default can be defined in the directory and not hard coded into the authenticator. It also makes a certain class of authenticator more indpendent because they might not have to make changes to a separate authentication server.

They can use SecurityManager in the hook I believe. The defaults will be exposed from the cache. Do you want SsoAuthenticator to decide to use or not the default in directory or we should force it after its response? This would mean as long as the customer returns true from SsoAuthenticator for successful login (could be even hardcoded return true) and the two defaults exist in directory, the web client will always spawn.

#191 Updated by Constantin Asofiei 9 months ago

Galya B wrote:

Greg Shah wrote:

I think the answer here is to provide a facility for the authenticator to get some configuration from the directory. Rather than require each authenticator to implement its own custom directory access, this would probably be best handled by a generic facility. Then this kind of default can be defined in the directory and not hard coded into the authenticator. It also makes a certain class of authenticator more indpendent because they might not have to make changes to a separate authentication server.

They can use SecurityManager in the hook I believe. The defaults will be exposed from the cache. Do you want SsoAuthenticator to decide to use or not the default in directory or we should force it after its response? This would mean as long as the customer returns true from SsoAuthenticator for successful login (could be even hardcoded return true) and the two defaults exist in directory, the web client will always spawn.

I don't think we should allow FWD to determine a default is required - the SSO authenticator code needs to decide, and explicitly use the default; hiding this in the FWD runtime can pose problems. FWD must always expect the authenticator to send all required data.

#192 Updated by Galya B 9 months ago

Constantin Asofiei wrote:

I don't think we should allow FWD to determine a default is required - the SSO authenticator code needs to decide, and explicitly use the default; hiding this in the FWD runtime can pose problems. FWD must always expect the authenticator to send all required data.

This sounds good to me. Do we feed the default user explicitly? If more directory configs are needed, they should find them themselves from the runtime classes (SecurityManager, Utils).
For the default OS account the currently existing config webClient/defaultAccount will be used. Do we need option for configuring a default os account pass?

#193 Updated by Galya B 9 months ago

And here is my conclusion on the other topic:
  • remotelaunchoption checks to be executed only for remote spawns (using WebClientLauncher network interface). Otherwise it doesn't make sense to require the configs.
  • trustedspawner to not be required for VD without SSO, because then we have the OS login screen and password is required and checked with PAM. It should be verified for all other cases: remote spawn, all SSO and embedded without SSO that runs on certificate + default fwduser.

#194 Updated by Galya B 9 months ago

Galya B wrote:

Do we feed the default user explicitly?

Actually my bad, it's already done in the latest revision with SsoAuthenticator.setOption and directory auth-mode/ssopluginoption. They can set anything they like in the option and parse it any way they want in the authenticator. The setter is called after instantiation.

#195 Updated by Galya B 9 months ago

Greg Shah wrote:

RPC WebClientLauncher.spawn with SSO

This still needs to be discussed. We have a customer that originally planned to have a separate web server with their own web application which would then embed our converted screens. I don't know if that approach is still needed. It was much more complicated than our current built-in approach.

The support actually will be quite simple: a new method spawnSso with the same params that will just add a flag isSso to spawn args, so that usr / pass will be treated as app user / pass. That's how it works now, I just need to add the new method to the interface. What about I add it and customers will have all the time they want to make up their minds?

#196 Updated by Greg Shah 9 months ago

The BrokerServerServices acl/net config described here seems outdated. Does it need to be removed from the document?

What is outdated about it? Without the net ACL, the broker API can't be exported.

#197 Updated by Greg Shah 9 months ago

RPC WebClientLauncher.spawn with SSO

This still needs to be discussed. We have a customer that originally planned to have a separate web server with their own web application which would then embed our converted screens. I don't know if that approach is still needed. It was much more complicated than our current built-in approach.

The support actually will be quite simple: a new method spawnSso with the same params that will just add a flag isSso to spawn args, so that usr / pass will be treated as app user / pass. That's how it works now, I just need to add the new method to the interface. What about I add it and customers will have all the time they want to make up their minds?

What you are proposing is not clear to me. I especially don't understand why we would pass a flag isSso to this API. The caller should not control the method of authentication (whether SSO is used or not). Perhaps we are talking about different things. I was talking about the current embedded mode approach where we have a session that is used to spawn clients, which is why we implemented the trustedspawner support.

#198 Updated by Galya B 9 months ago

I need confirmation that:
1. ssopluginoption is enough to support a default fwd user.
2. webClient/defaultAccount will be used to support a default os account. Confirmation if that will be always running on trusted or it needs a password optionally.
3. confirmation on #3931-193

#199 Updated by Galya B 9 months ago

Greg Shah wrote:

RPC WebClientLauncher.spawn with SSO

What you are proposing is not clear to me. I especially don't understand why we would pass a flag isSso to this API.

OK, then just the javadoc needs to be changed to reflect the fact that with SSO enabled, usr / pw is not for os, but in-app user / pass.

The caller should not control the method of authentication (whether SSO is used or not).

Of course the caller is not controlling it. It was going to fail, if SSO is not enabled on the server. But I thought it's good that they know SSO is enabled and the type of params is different.

I was talking about the current embedded mode approach where we have a session that is used to spawn clients, which is why we implemented the trustedspawner support.

I have no idea what you're talking about. I don't see forking session from session anywhere in embedded currently. trustedspawner checks are activated only on one condition - os account without pass.

#200 Updated by Galya B 9 months ago

Can you share client configs for <security> and <access> for a broker running against testcases server?

#201 Updated by Greg Shah 9 months ago

1. ssopluginoption is enough to support a default fwd user.

This is what I suggested previously, so I tend to agree.

2. webClient/defaultAccount will be used to support a default os account.

OK

Confirmation if that will be always running on trusted or it needs a password optionally.

We need the option of having a password. If I recall correctly, Windows requires it.

#202 Updated by Greg Shah 9 months ago

remotelaunchoption checks to be executed only for remote spawns (using WebClientLauncher network interface). Otherwise it doesn't make sense to require the configs.

Are you suggesting moving the check out of WebHandler.spawnWorker()/WebHandler.verifySpawnOptions()?

trustedspawner to not be required for VD without SSO, because then we have the OS login screen and password is required and checked with PAM.

Roughly yes, but I worry that you are too focused on trustedspawner which brings with it the assumption that we already have a session.

To be clear: trustedspawner itself is not the more important thing. The result of what we achieved with trustedspawner is the important thing. So we can do that using a different approach than trustedspawner, so long as we don't break embedded mode.

It should be verified for all other cases: remote spawn, all SSO and embedded without SSO that runs on certificate + default fwduser.

I mostly care about the result which has an option to avoid the OS login screen.

#203 Updated by Greg Shah 9 months ago

Galya B wrote:

Greg Shah wrote:

RPC WebClientLauncher.spawn with SSO

What you are proposing is not clear to me. I especially don't understand why we would pass a flag isSso to this API.

OK, then just the javadoc needs to be changed to reflect the fact that with SSO enabled, usr / pw is not for os, but in-app user / pass.

Good. I prefer that.

The caller should not control the method of authentication (whether SSO is used or not).

Of course the caller is not controlling it.

Passing the parameter suggests that the caller has some control. If they don't then it is just confusing.

I was talking about the current embedded mode approach where we have a session that is used to spawn clients, which is why we implemented the trustedspawner support.

I have no idea what you're talking about. I don't see forking session from session anywhere in embedded currently. trustedspawner checks are activated only on one condition - os account without pass.

The current use of trustedspawner is a kind of "forking" because it enables spawning from an existing session.

#204 Updated by Galya B 9 months ago

Greg Shah wrote:

remotelaunchoption checks to be executed only for remote spawns (using WebClientLauncher network interface). Otherwise it doesn't make sense to require the configs.

Are you suggesting moving the check out of WebHandler.spawnWorker()/WebHandler.verifySpawnOptions()?

Even originally it wasn't executed always, but on condition. My idea is to be checked only when it's relevant: on remote launch attempt (I made it so in the latest revision).

Roughly yes, but I worry that you are too focused on trustedspawner which brings with it the assumption that we already have a session.

We have a security context to be able to check if the fwd user has the permissions. Otherwise defining the resource in directory is pointless. I couldn't understand why it exists, that's why the whole conversation about it. At the time a session exists, trustedspawner is even more irrelevant.

The current use of trustedspawner is a kind of "forking" because it enables spawning from an existing session.

I can't see that in the code. All I see is remote / web requests come with os account without password to spawn in "trusted", which is an int sent to the spawner to bypass PAM.

#205 Updated by Galya B 9 months ago

Greg Shah wrote:

The current use of trustedspawner is a kind of "forking" because it enables spawning from an existing session.

All web requests for embedded just spawn a new client based on the certificate with the associated fwd user and the default os user for web in directory.

#206 Updated by Galya B 9 months ago

Here are the mockups for the Admin UI: 1. OS users (new page) 2. OS user password update (new dialog), 3. FWD user edit to associate with OS user (new field). Keeping it quite simple and consistent with the current ui.

1.
2.
3.

Please review and confirm if this is what is needed for now.

#207 Updated by Galya B 9 months ago

The default os user password will have to be stored as env var, it can't be entered as plain text in webClient configs.

#208 Updated by Greg Shah 9 months ago

remotelaunchoption checks to be executed only for remote spawns (using WebClientLauncher network interface). Otherwise it doesn't make sense to require the configs.

Are you suggesting moving the check out of WebHandler.spawnWorker()/WebHandler.verifySpawnOptions()?

Even originally it wasn't executed always, but on condition. My idea is to be checked only when it's relevant: on remote launch attempt (I made it so in the latest revision).

I prefer not to add additional conditions. Even the existing condition creates a security hole that I wish was not there. The more conditions we add, the more we have to think about how we use it and the more likely a real security problem is introduced by accident.

Roughly yes, but I worry that you are too focused on trustedspawner which brings with it the assumption that we already have a session.

We have a security context to be able to check if the fwd user has the permissions. Otherwise defining the resource in directory is pointless. I couldn't understand why it exists, that's why the whole conversation about it. At the time a session exists, trustedspawner is even more irrelevant.

The current use of trustedspawner is a kind of "forking" because it enables spawning from an existing session.

I can't see that in the code. All I see is remote / web requests come with os account without password to spawn in "trusted", which is an int sent to the spawner to bypass PAM.

The fact that you are in a context now and spawn another context is what I am calling "forking". Yes, it is not obvious at all. And I would like to eliminate it as noted above. The context that is there for embedded mode is historical and is not actually needed in the current target environments. We are not even sure it is needed for the original customer so I really don't want to keep that complication.

#209 Updated by Greg Shah 9 months ago

The default os user password will have to be stored as env var, it can't be entered as plain text in webClient configs.

We must not do that. Environment variables can be read through the /proc/ filesystem which means that anything in the environment is public information.

That is why we have a complex pipe approach used to deliver the password to the spawner, because pipes are private transfer mechanisms.

#210 Updated by Galya B 9 months ago

Greg Shah wrote:

The current use of trustedspawner is a kind of "forking" because it enables spawning from an existing session.

I can't see that in the code. All I see is remote / web requests come with os account without password to spawn in "trusted", which is an int sent to the spawner to bypass PAM.

The fact that you are in a context now and spawn another context is what I am calling "forking". Yes, it is not obvious at all. And I would like to eliminate it as noted above. The context that is there for embedded mode is historical and is not actually needed in the current target environments. We are not even sure it is needed for the original customer so I really don't want to keep that complication.

Wait, wait, I'm not an idiot, having that assumption prevents you from understanding what I'm doing, looking at the code would explain it, putting it in words takes days before we get on the same page. There is no context for web requests - the main spawn method. For remote spawns there is context and no context is created. The "historical" context is the only way to check the security configs and you seem to want in any possible case to do the checks according to the other statement:

I prefer not to add additional conditions. Even the existing condition creates a security hole that I wish was not there. The more conditions we add, the more we have to think about how we use it and the more likely a real security problem is introduced by accident.

#211 Updated by Greg Shah 9 months ago

In regard to the proposed UI changes, I think 1 and 3 are good to go. For 2, I would prefer if it is more obvious that it is editing the OS User instead of a FWD User. I think this may just need a different title to make that clear.

#212 Updated by Galya B 9 months ago

Greg Shah wrote:

That is why we have a complex pipe approach used to deliver the password to the spawner, because pipes are private transfer mechanisms.

There is no approach to deliver os user passwords to the spawner. VD uses a plain text, nonhashed pass coming from the web page, it's delivered as plain text. Embedded doesn't use a password.

For FWD users there is a hashed password, but it's entered through the admin ui and that's why it's hashed. How can we have the default os account with a hashed password that should be directly entered in directory.

#213 Updated by Galya B 9 months ago

Greg Shah wrote:

In regard to the proposed UI changes, I think 1 and 3 are good to go. For 2, I would prefer if it is more obvious that it is editing the OS User instead of a FWD User. I think this may just need a different title to make that clear.

I've missed to update the title in two. On Monday I'll start working on the changes.

#214 Updated by Galya B 9 months ago

Greg Shah wrote:

The more conditions we add, the more we have to think about how we use it and the more likely a real security problem is introduced by accident.

Actually the whole topic has been started, because I want the security checks to be enabled for VD as well, where they were not working at all. Configuring remote options for web spawn doesn't make sense, while I was configuring them to be able to run the code I was confused. The whole code for these options is completely in the hands of the FWD developer and it doesn't make sense the customer to support these configs without explicitly using them for remote spawns.

#215 Updated by Greg Shah 9 months ago

That is why we have a complex pipe approach used to deliver the password to the spawner, because pipes are private transfer mechanisms.

There is no approach to deliver os user passwords to the spawner.

Please see the password() function of spawn.c which reads the password from STDIN which is a pipe. We must avoid passing the plain text via the environment AND must avoid passing plain text password via the command line parameter (which also can be read publically via the procfs.

#216 Updated by Galya B 9 months ago

Greg Shah wrote:

That is why we have a complex pipe approach used to deliver the password to the spawner, because pipes are private transfer mechanisms.

There is no approach to deliver os user passwords to the spawner.

Please see the password() function of spawn.c which reads the password from STDIN which is a pipe. We must avoid passing the plain text via the environment AND must avoid passing plain text password via the command line parameter (which also can be read publically via the procfs.

Yes, the spawner takes it via the stream as plain text, but in VD it comes from web as plain text that is actually more disturbing than communication between processes on the same machine.
But I was speaking about where this password comes from. There is no mechanism to read the os password from directory.

#217 Updated by Galya B 9 months ago

Greg Shah wrote:

The fact that you are in a context now and spawn another context is what I am calling "forking".

There is no such mechanism nowhere: POST /gui, POST /chui, /embedded/launch, WebClientLauncher.spawn logic. No session is created, just a process is built to spawn the client and the client process creates the session with GuestAccess authentication.

Galya B wrote:

For remote spawns there is context and no context is created.

Actually this context is most likely going to fail the security checks if they are properly implemented. The remote host creates a session based on certificate and a default fwd user. It works fine now, but for SSO the fwd user is not related to the certificate / it's dynamic, so this check will most definitely fail if properly implemented. Luckily I think it's not fully implemented, it doesn't seem to check the actual subject, at least as far as I can read the code. But let's not water down the discussion, this is a side story for another day.

#218 Updated by Galya B 9 months ago

OS passwords can't be stored in directory.xml the same way as FWD user passwords. FWD passwords are encrypted with SHA, which doesn't allow decryption. This works because it's used only for verification of entered values against the encrypted pass. But OS user password needs to actually be read in plain text to be fed to PAM.

What about storing the OS pass in directory as base64 encrypted with salt. The salt can be a server cmd line param and how it's stored and retrieved will be up to the customer. If the salt needs to be changed, there will be a second server cmd line param for the new salt; on receiving the second param all passwords will be decrypted with the old salt, encrypted with the new and stored.

Any other ideas?

#219 Updated by Galya B 9 months ago

I think #30 can be resolved with #3931. If the customer wants to prevent this type of DOS, they can replace their 4GL login screen with the FWD's one.

#220 Updated by Greg Shah 9 months ago

By #30 do you mean #3931-30?

#221 Updated by Galya B 9 months ago

Greg Shah wrote:

By #30 do you mean #3931-30?

Nope, #30. This was mentioned by Constantin in response to my email about DOS, if you remember. Now that I took a look, it seems a problem solvable by moving to login outside 4GL.

#222 Updated by Greg Shah 9 months ago

Galya B wrote:

Greg Shah wrote:

By #30 do you mean #3931-30?

Nope, #30. This was mentioned by Constantin in response to my email about DOS, if you remember. Now that I took a look, it seems a problem solvable by moving to login outside 4GL.

Yes, agreed.

#223 Updated by Galya B 9 months ago

Do we support remote network interface for launching clients only for embedded? Currently it's enabled for any web client. If a customer has only VD, do we need to open this interface?

#224 Updated by Greg Shah 9 months ago

Do we support remote network interface for launching clients only for embedded? Currently it's enabled for any web client. If a customer has only VD, do we need to open this interface?

Yes. And I am wondering if we even really need to support that for Embedded mode.

#225 Updated by Galya B 9 months ago

It's more secure, but more complicated. I can't think of a reason to remove it, since it's already there, but it may need a more explicit config to be enabled.

#226 Updated by Galya B 9 months ago

The other way of spawning embedded needs some sort of authentication. Do we tackle this?

#227 Updated by Greg Shah 9 months ago

It's more secure, but more complicated. I can't think of a reason to remove it, since it's already there, but it may need a more explicit config to be enabled.

Being more complicated is a good enough reason to remove it, if we really don't need it. I can't say that for sure, but I don't know that we do need it.

The other way of spawning embedded needs some sort of authentication. Do we tackle this?

The original idea was that we would tackle this so that the result would be simpler. I was also hoping that there would be quite a bit of common code in both embedded and virtual desktop approaches.

#228 Updated by Galya B 9 months ago

In directory we have the following configs related to embedded:
  • embeddedWebApp/enabled (boolean) used in StandardServer.bootstrap to initialize embedded handlers;
  • webClient/embedded (boolean) read in WebClientBuilderOptions.readOptions and passed forward to the client / driver;
  • webClient/enabled (boolean) used in StandardServer.bootstrap to register the network interface WebClientLauncher.

#229 Updated by Galya B 9 months ago

Greg Shah wrote:

The other way of spawning embedded needs some sort of authentication. Do we tackle this?

The original idea was that we would tackle this so that the result would be simpler. I was also hoping that there would be quite a bit of common code in both embedded and virtual desktop approaches.

There is. I added a base class for both type of handlers, so the code is reused as much as possible, but here are a few differences:

  • Embedded uses a certificate and relevant configs (alias) from directory to create the security context for checking the security configs before spawning. I've created an alternative SecurityManager method to create a context only based on a FWD account, so I can remove the use of the certificate and change the configs to define only the FWD user.
  • Embedded has a different index.html startup page and additional handlers for a list of static resources (js libs, scripts, etc). In #3931-137 it got confirmed that all customers will migrate to SSO, so I'll have to merge the index.html and the SSO login page.
  • Both web handlers load / serve static res differently. Embedded loads the file in-memory and then decides to return 404 or 200, while VD writes line by line and doesn't return 404. Here I think we'll need a hybrid approach: if the res file is not found (or any IOException occurs), return 404, otherwise even if the file is empty, return 200 and write line by line.
  • Embedded spawns new clients on rpc calls. Removing the network interface will simplify a little bit the logic and can be added back later if needed, but I need a confirmation if we want try that.
  • Embedded spawns new clients on request to /embedded/launch with any HTTP method and allows CORS, while VD handles only POST /gui (and I removed the CORS successfully). Since the current customers don't use embedded in production, we can move it to the same approach, but it will probably break their code initially. Also I need confirmation that their login page will always be served by the FWD server instead of hosted somewhere else. Also the embedded handler will need a way to be configured to serve a custom page.

#230 Updated by Galya B 9 months ago

Constantin, can you tell if these handlers are actually used by any embedded index.html or only by report in p2j/report/web/res/index.html:

            new DojoToolkitHandler(basePath),
            new WebResourceHandler(basePath, "/bootstrap", "bootstrap-3.3.7-dist"),
            new WebResourceHandler(basePath, "/d3", "d3-4.9.1"),
            new WebResourceHandler(basePath, "/fonts", "fonts"),
            new WebResourceHandler(basePath, "/jquery", "jquery-3.2.1"),
            new WebResourceHandler(basePath, "/jquery-ui", "jquery-ui-1.12.1.custom"),
            new WebResourceHandler(basePath, "/material-components-web", "material-components-web"),
            new WebResourceHandler(basePath, "/material-design-icons", "material-design-icons"),
            new WebResourceHandler(basePath, "/tabulator", "tabulator-master-2.12.0")

#231 Updated by Constantin Asofiei 9 months ago

Galya, the FWD analytics (which I assume you mean by 'report') is managed by p2j/report/server/ReportWebServer. All the handlers you mentioned are default resources which are used in the embedded web app.

#232 Updated by Galya B 9 months ago

These embedded resources are really confusing, but I think I start understanding what is going on.

The FWD server serves:
  • the index page from the resources of the custom package root;
  • the libraries from zip files that somehow end up in deploy/lib and get mapped to web contexts by AbstractModulesHandler. The zip files are available on the FWD artifact repo and listed in javascript_artifacts-front_end.xml, but what does it have to do with the build script is still unclear;
  • the actual login logic / js scripts from the web gui driver's resources p2j/ui/client/gui/driver/web/res/ (I would expect to find them on the client embedded server only).

I've also finally found the answer of my question in #3931-20: the login function in p2j/ui/client/gui/driver/web/res/p2j.emain.js is called by a script in the the customer's index.html.

#233 Updated by Galya B 9 months ago

Do we want to support a default login page for embedded when the customer doesn't provide one? Currently we don't.

#234 Updated by Galya B 9 months ago

If there is no FWD login page it's the same as with the custom VD login - some features won't be available before starting the client or at all like browser fingerprint, client-side registry and persistent config storage.

#235 Updated by Greg Shah 9 months ago

Do we want to support a default login page for embedded when the customer doesn't provide one? Currently we don't.

Yes, this is a good idea.

If there is no FWD login page it's the same as with the custom VD login - some features won't be available before starting the client or at all like browser fingerprint, client-side registry and persistent config storage.

I like it.

#236 Updated by Galya B 9 months ago

Is the theme selector needed for embedded? Are there themes in embedded? Currently only virtual desktop sets the option.

#237 Updated by Greg Shah 9 months ago

This selector is a convenience only. It is not strictly needed in any mode, however it is just as useful in both modes. At some point we probably need to provide a more generic facility for the user to provide custom inputs for the client (which then would need to be secured before honoring).

#238 Updated by Greg Shah 9 months ago

The key point here is that the logins can be the same.

#239 Updated by Galya B 9 months ago

Greg Shah wrote:

The key point here is that the logins can be the same.

Embedded includes all the driver resources in the login page and then serves the client in the frame, while VD won't need these libraries and scripts and they don't have to be exposed, so the pages will be different resources, but the looks will be the same.

#240 Updated by Galya B 9 months ago

I'm not sure how these two evolved and ended up being so different. I guess they can be unified further, but I'm not confident in what should this entail. I guess instead of loading the gui driver resources from the FWD server and instead of having a heavy login page for embedded, both modes can have a lightweight login page with iframe. No redirects for both (anyways that was planned for later to achieve consistent origin). But it's probably not so straight forward shifting in that direction. On the other hand now it may be the best time to do it, instead of creating a separate login page for embedded and before allowing a custom login page for VD (it can be served in an iframe allowing all FWD customizations).

#241 Updated by Greg Shah 9 months ago

Yes, that makes sense.

#242 Updated by Greg Shah 9 months ago

To be clear: I'm OK with reworking things now to achieve more common code and a consistent approach.

#243 Updated by Galya B 9 months ago

OK, I'll be digging deeper.

#244 Updated by Galya B 9 months ago

Embedded doesn't have a face, because it's supposed to be shaped by the customer, but can I summarize what it is with: it draws widgets the same way as web gui, but has some of them removed to allow custom js-based frame in the parent container? Also there is the js toolset for developing the UI in the container.

But I don't understand the integration of the front-end. The driver resources (p2j.emain.js) depend on elements in the DOM tree (probably other dependencies as well), at the same time there is not a default page. Do we have a documentation on what is the contract of the core scripts? Do we expect the customers to be able to create their own pages or not (I'm not sure about the business model, that's a valid question in that regard)?

#245 Updated by Galya B 9 months ago

Constantin, do you know if hotel_gui's embedded works? With latest trunk + latest hotel_gui I get a very weird behavior and I'm not sure if it's expected. Any credentials get me through the login window and then I get CORS error.

I had a working old version, but then I updated. I made sure to clean rebuild, clean browser cache, restart browser/pc/server... it's still not working. I wanted to compare my branch to trunk, now I don't know what's going on.

#246 Updated by Constantin Asofiei 9 months ago

Embedded mode with Hotel GUI rev 271 and FWD trunk rev 14703 works fine.

#247 Updated by Galya B 9 months ago

It works, it's just that the CORS error is actually present in the console and some DB configs are missing in directory.xml.template and no relevant error appears to the user or in the logs + the 30 sec delay I fixed in my branch but still present on trunk; all of it made it misleading it's not working.

#248 Updated by Galya B 9 months ago

Hotel gui works well in a parent frame with login form validated by SsoAuthenticator. Even the CORS error (a non-critical one present in the console) disappeared. But there is trouble.

The hotel gui 4GL app does another login in emain.p after the client is spawned:

procedure doLogin.
   def input param messageId as int.
   def input param username as char.
   def input param password as char.

   if setuserid(username, password)
   then do:
      fwdInitialized = true.
      P2J-REMOTE-CALL('~{ "api":"p2jInitialized" ~}').
   end.
   else do:
      P2J-REMOTE-CALL('~{ "api":"showError" , "error":"Invalid username or password." ~}').
      quit.
   end.
end.

This one doesn't work with SSO. It uses the same user / pass from the login page and tries to map them to a FWD user / pass from directory.

We discussed that the app receives SESSION:AUTH-BLOB and validates its in-app user credentials against 3rd party auth server. Do we need to support something else for hotel gui? Can you please be more specific.

#249 Updated by Galya B 9 months ago

So setuserid does DB auth, which is a valid approach. I haven't uploaded anything to hotel_gui, but there are certain updates I did locally to configs and a new SsoAuthenticator to handle the necessary changes. I confirmed the new branch will be working with hotel gui, but it will need some adjustments. After 3931a is merged, we can get back to it, I'll probably need some help with the ui.

I'll move on with some more javadoc missing in the new classes and then #4571.

#250 Updated by Greg Shah 9 months ago

Galya: Along with other objectives, Hotel GUI is intended to be an example of how to implement Embedded Mode for a GUI application. I would expect to make any needed changes to implement a simple, clean and consistent approach for authentication. That would include moving the 4GL authentication code into a pure Java SsoAuthenticator approach.

Constantin: Please review the recent changes and the discussion related to embedded mode.

#251 Updated by Greg Shah 9 months ago

To be clear: in #3770-81 I noted that our current login approach with embedded mode does need serious changes. Spawning before login is not OK. The currrent approach is a result of the historical fact that it was first prototyped using a separate web server (that had its own persistent FWD session) AND was also a result of some short term time pressure to get a POC written. We are now dealing with that technical debt.

#252 Updated by Galya B 9 months ago

The hotel gui js and the embedded driver js (intertwined with it) are very particular. I'm sure we don't have a guide how to run and debug the js code without redeploying and using console.log. I'm afraid that will be a black whole for my time, if I need to handle it myself.

Constantin won't be able to run hotel gui at the moment without configs and code from my side. Please tell, if needed.

#253 Updated by Galya B 9 months ago

Greg Shah wrote:

To be clear: in #3770-81 I noted that our current login approach with embedded mode does need serious changes. Spawning before login is not OK.

Actually this is already fixed in 3931a. It's just that the second login coming from 4GL is still present and it works without SSO.

#254 Updated by Galya B 9 months ago

But I think it's fine. "works without SSO" means it directly checks the user / pass against the DB, where the in-app users are stored. In the new world it's supposed to check the AUTH-BLOB and not have access to the user / pass directly. I definitely see place for improvement in the js code as it developed freely and have some downsides. And those changes can remove the direct access to user / pass, but the same values can be passes in to the app again through the AUTH-BLOB and the flow will not be changed.

What I'm trying to say is the major flaw with launching before auth is fixed. I have some troubles with css / hidden / visible windows / dialogs that changed due to the login page using these directly now and I will need help, but other than that major refactoring is avoidable.

#255 Updated by Galya B 9 months ago

A question about configs: currently if auth-mode/ssoplugin is defined and can be instantiated, then SSO is enabled. Otherwise virtual desktop falls back to os user / pass login (which is still a supported mode) and embedded is disabled (no handlers registered, 404 served). The question is if auth-mode/ssoplugin is defined, but can't be instantiated (some typo in the package, class name), does it fallback to os user / pass login or stops server initialization?

#256 Updated by Greg Shah 9 months ago

What I'm trying to say is the major flaw with launching before auth is fixed.

Yes, I understand. My discussion was just to clarify the history.

As part of our work, I do want us to move from the embedded mode 4GL login code to the Java SSO approach. This will also be wanted to remove the virtual desktop approach login dialog.

#257 Updated by Greg Shah 9 months ago

The question is if auth-mode/ssoplugin is defined, but can't be instantiated (some typo in the package, class name), does it fallback to os user / pass login or stops server initialization?

We should log the failure and fallback to the default approach.

#258 Updated by Galya B 9 months ago

Greg Shah wrote:

What I'm trying to say is the major flaw with launching before auth is fixed.

Yes, I understand. My discussion was just to clarify the history.

As part of our work, I do want us to move from the embedded mode 4GL login code to the Java SSO approach. This will also be wanted to remove the virtual desktop approach login dialog.

There are two phases of the authentication process, roughly so: 1. login page with form POST request to authenticate in-app user/pass with the Java hook. 2. 4GL verification of SESSION:AUTH-BLOB, which is very customer specific.

Virtual desktop is ready to work with one login since it supports both phases. The same goes for embedded after the changes I have introduced.

Only the code in hotel gui being a sample needs some tweaking in the css department. Now the doLogin logic in 4GL will still be needed to verify the SESSION:AUTH-BLOB instead of as it now receives the same user/pass from the front-end. I mean the current approach is not wrong, just a bad example for a cleaner solution.

Also what I mean is the framework's code is done.

#259 Updated by Galya B 9 months ago

I was thinking how to protect better the expensive resource of spawning a new web client process. And I was thinking about CSRF tokens, captcha, cookies, etc. But at the end of the day I think the most important part is license management and good logs that provide info who is the abuser. The abuser is someone with valid username and password, so it's very much in the hands of the customers to keep that in-check. And of course it's important to disable the login button on click to keep away neurotic end-users from unintentionally committing offense. With allowing auto-login, as I've said in the past, new clients will be way easier to get spawned, so customers need to be aware that more precaution is expected on their end.

#260 Updated by Galya B 9 months ago

We can further expand the args passed in to the SsoAuthenticator to allow licensing management, but that with next tasks.

#261 Updated by Galya B 9 months ago

I get:

FINE | TranslationManager | ThreadName:Conversation [00000003:hotel], Session:00000003, Thread:00000019, User:hotel | Failed to apply language for referent 'com.goldencode.hotel.Emain'. The referent wasn't initialized.

Does it mean the app wasn't initialized?

#262 Updated by Greg Shah 9 months ago

The abuser is someone with valid username and password, so it's very much in the hands of the customers to keep that in-check.

Agreed.

With allowing auto-login, as I've said in the past, new clients will be way easier to get spawned, so customers need to be aware that more precaution is expected on their end.

Yes. We should help here by making auto-login support disabled by default. I want customers to have to do something to enable it.

We can further expand the args passed in to the SsoAuthenticator to allow licensing management, but that with next tasks.

Agreed.

#263 Updated by Galya B 9 months ago

Greg Shah wrote:

With allowing auto-login, as I've said in the past, new clients will be way easier to get spawned, so customers need to be aware that more precaution is expected on their end.

Yes. We should help here by making auto-login support disabled by default. I want customers to have to do something to enable it.

To enable auto-login they need to implement two methods of the SsoAuthenticator: getAuthCookieName and Result authenticate(Cookie[] cookies, String clientIp). If one of these returns null, auto-login will be disabled.

#264 Updated by Greg Shah 9 months ago

Galya B wrote:

Greg Shah wrote:

With allowing auto-login, as I've said in the past, new clients will be way easier to get spawned, so customers need to be aware that more precaution is expected on their end.

Yes. We should help here by making auto-login support disabled by default. I want customers to have to do something to enable it.

To enable auto-login they need to implement two methods of the SsoAuthenticator: getAuthCookieName and Result authenticate(Cookie[] cookies, String clientIp). If one of these returns null, auto-login will be disabled.

Perfect. Does getAuthCookieName have a default version that returns null?

#265 Updated by Galya B 9 months ago

Greg Shah wrote:

To enable auto-login they need to implement two methods of the SsoAuthenticator: getAuthCookieName and Result authenticate(Cookie[] cookies, String clientIp). If one of these returns null, auto-login will be disabled.

Perfect. Does getAuthCookieName have a default version that returns null?

I haven't implemented a base class. There is a sample one, but it's with dummy values, so basically it's up to their implementation. It says in the JavaDoc of the authenticate method it's for auto-login.

#266 Updated by Galya B 9 months ago

Also the hotel gui SsoAuthenticator implementation (I still haven't uploaded) returns null.

#267 Updated by Greg Shah 9 months ago

I was thinking of a default method in the interface.

#268 Updated by Galya B 9 months ago

Ah, this makes sense. Up in r14683.

#269 Updated by Galya B 9 months ago

  • % Done changed from 0 to 100
  • Status changed from WIP to Review

3931a rebased on trunk r14707. Ready for review.

#270 Updated by Galya B 9 months ago

P.S. File headers in the making.

#271 Updated by Constantin Asofiei 9 months ago

I'm reviewing 3931a.

#272 Updated by Galya B 9 months ago

r14755 with file headers.

Constantin, if you can, check it out. It will be helpful. There are a bunch of files updated with SecurityManager context methods calls updated.. that once reviewed, can be ignored further down the line.

#273 Updated by Galya B 9 months ago

Final touch-ups in r14756: CertificateUtil removed, file headers added to AccountExtUtil and SecurityUtil.

#274 Updated by Galya B 8 months ago

Here is what needs to be done to hotel_gui to run with 3931a:

Add the SsoAuthenticator implementation (dummy as of now) in src/com/goldencode/hotel:

package com.goldencode.hotel;

import com.goldencode.p2j.security.*;

import javax.servlet.http.*;
import java.util.*;

public class HotelGuiSsoAuthenticator
implements SsoAuthenticator
{
   @Override
   public void setOption(String s)
   {
   }

   @Override
   public String getLoginPage(String s)
   {
      // defaults to the custom resource
      return null;
   }

   @Override
   public Result authenticate(Map<String, String[]> map, Cookie[] cookies, String s)
   {
      return new Result(true, "bogus", null, null);
   }
}

(To make this authenticator more realistic, it needs to query the DB the same way as doLogin in 4GL and verify the user/pass and accept the default fwd user from option.)

In embedded/src/resources rename index.html to web_partial_login.html and in build.xml find and rename the one occurrence.

In directory:
  • in config/auth-mode add <node-attribute name="ssoplugin" value="com.goldencode.hotel.HotelGuiSsoAuthenticator"/>;
  • add a container accounts/osusers with one child <node class="osuser" name="[os-user]"/> for trusted more. You can set password in Admin UI;
  • (optional - this one or the next one) in accounts/users/bogus with an attr <node-attribute name="osuser" value="[os-user]"/>;
  • (optional - this one or the previous one) rename webClient/defaultAccount to webClient/defaultOsUser and set a value for [os-user] if not present;
  • rename webClient/virtualDesktopEnabled -> webClient/virtualDesktop;
  • remove webClient/enabled;
  • change value of webClient/embedded to TRUE;
  • remove embeddedWebApp container (embeddedWebApp/cfgOverrides can be moved to webClient/cfgOverrides, but these are not present for hotel_gui);
  • if no password is specified for the osuser, then add subject bogus for trustedspawner;

#275 Updated by Galya B 8 months ago

A few minor fixes pushed to r14757.

#276 Updated by Galya B 8 months ago

r14758 adds support for /webres context for base virtual desktop and embedded resources (style.css, login page, favicon.ico and different .png / .svg images).

Build script update for testcases:

   <property name="app.webres.dir" value="webres" />

   <target name="prepare">

     [...]

      <!-- copy base web res dir -->
      <copy todir="${build.home}/classes/${pkgroot}">
         <fileset dir="${p2j.home}">
            <include name="${app.webres.dir}/**"/>
         </fileset>
      </copy>

     [...]
   </target>

In the previous revisions replacing the login page with a custom one was already supported for both virtual desktop and embedded. What this revision achieves is to standardize the resource location in the custom package for both modes. Also supports handling / serving of the additional commonly requested files.

As an end result the html and svg files from #7067 can be directly placed in webres dir in the root folder of the project and the page will be up to the customer's vision. I modified them slightly and attach the folder here to be used for testing with testcases. The contract between a custom page and FWD can be documented in a wiki.

Next: Rework the default template to allow styling only by providing a css file and remove the directory configs.

#277 Updated by Galya B 8 months ago

r14759 an updated default login screen (html + css). Desktop settings (bgr attributes from directory) removed.

The default login is kept simple, still using browser styling for select and some input attrs. On landscape with mobile overflow:hidden allows scrolling. I tried to keep the new html + css as lean/clean as possible.

The attached archive can be unzipped in testcases root to achieve the same look.

#278 Updated by Constantin Asofiei 8 months ago

Galya, this is the first part of the review. I'm still going through the SSO changes and the SecurityManager refactoring.
  1. AUTH_BLOB
    • check and fix conversion for h:auth-blob support (i.e. handle access, not SESSION access)
  2. OS User Admin
    • AdminServiceImpl.setOsUser - if plainPassword is null/empty, don't we need to reset password?
    • the OS user password is just hashed base64-encoded. We need it in clear-text for i.e. Windows; Greg, do we need to think for a way to encrypt this?
    • maybe mark somehow users with a password set? (via a check column in the list?)
    • deleting an OS user - we need an alert or maybe just not allow to delete an OS user if one or more FWD users is associated with it
    • SecurityAdmin - listOsUsers and getOsUser have ADLU_PASSWORD_ACCESS, ADLU_READ_ACCESS, ADLU_WRITE_ACCESS; as password is not displayed on the admin UI, then do not transfer it and use only ADLU_READ_ACCESS.
    • <node class="container" name="osusers" /> needs to be manually added, to be able to add a 'first' OS user; please add this node automatically.
    • when selecting an OS user on the left side, right-side needs to be a list of all FWD users associated with this OS user (something similar to Accounts/Groups view)
    • OS usernames are case-sensitive under linux, but not on Windows. SecurityCache.getOsUserById (and others) assumes case-insensitive. I don't think this will be an issue, but it's worth mentioning.
    • SecurityCache.readAuthMode - see line 2108, for Loaded sso auth hook - you are using hookName and we need NPE protection for using ssoHookName.

#279 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

  • check and fix conversion for h:auth-blob support (i.e. handle access, not SESSION access)

In previous comments we were discussing SESSION:AUTH-BLOB, why handle?

  • AdminServiceImpl.setOsUser - if plainPassword is null/empty, don't we need to reset password?

Maybe an improvement, but it should be working now as well. When empty, it will get into trusted mode.

  • the OS user password is just hashed base64-encoded. We need it in clear-text for i.e. Windows; Greg, do we need to think for a way to encrypt this?

It is not hashed, it is encrypted with our custom base64 and then translated to byte array (the type BYTEARRAY for dir_schema) and stored in directory. Reversed to plain text, when needed by the spawner.

  • maybe mark somehow users with a password set? (via a check column in the list?)
  • deleting an OS user - we need an alert or maybe just not allow to delete an OS user if one or more FWD users is associated with it
  • SecurityAdmin - listOsUsers and getOsUser have ADLU_PASSWORD_ACCESS, ADLU_READ_ACCESS, ADLU_WRITE_ACCESS; as password is not displayed on the admin UI, then do not transfer it and use only ADLU_READ_ACCESS.
  • <node class="container" name="osusers" /> needs to be manually added, to be able to add a 'first' OS user; please add this node automatically.
  • when selecting an OS user on the left side, right-side needs to be a list of all FWD users associated with this OS user (something similar to Accounts/Groups view)
  • OS usernames are case-sensitive under linux, but not on Windows. SecurityCache.getOsUserById (and others) assumes case-insensitive. I don't think this will be an issue, but it's worth mentioning.

All of these are good improvements. Will be added.

  • SecurityCache.readAuthMode - see line 2108, for Loaded sso auth hook - you are using hookName and we need NPE protection for using ssoHookName.

I'll fix the message, but I don't see possible NPE.

#280 Updated by Constantin Asofiei 8 months ago

Galya B wrote:

Constantin Asofiei wrote:

  • check and fix conversion for h:auth-blob support (i.e. handle access, not SESSION access)

In previous comments we were discussing SESSION:AUTH-BLOB, why handle?

For consistency; you can have:

def var h as handle.
h = SESSION.
message h:auth-blob.

  • SecurityCache.readAuthMode - see line 2108, for Loaded sso auth hook - you are using hookName and we need NPE protection for using ssoHookName.

I'll fix the message, but I don't see possible NPE.

I mean this code:

      ssoHookName = ds.getNodeString("/security/config/auth-mode", "ssoplugin");
      ssoHookOption = ds.getNodeString("/security/config/auth-mode", "ssopluginoption");
      dList("Loaded sso auth hook as <" + ssoHookName + ">");

will log 'null' as the hook name. maybe use a different text (No SSO auth hook specified).

#281 Updated by Greg Shah 8 months ago

Galya B wrote:

r14759 an updated default login screen (html + css). Desktop settings (bgr attributes from directory) removed.

The default login is kept simple, still using browser styling for select and some input attrs. On landscape with mobile overflow:hidden allows scrolling. I tried to keep the new html + css as lean/clean as possible.

I like the look and feel of the default. It is nice and clean.

Sergey: Please review the changes.

The attached archive can be unzipped in testcases root to achieve the same look.

If this is the default login for FWD, why do we need to add the logo and favicon to the application jar? I'd prefer if the default was available from the p2j.jar just like the rest of our core web resources.

#282 Updated by Greg Shah 8 months ago

Let's also make sure that the new login and the error message processing is honoring our I18N translations approach.

Sergey: Add that to your review.

#283 Updated by Galya B 8 months ago

Greg Shah wrote:

If this is the default login for FWD, why do we need to add the logo and favicon to the application jar? I'd prefer if the default was available from the p2j.jar just like the rest of our core web resources.

I wasn't sure if it makes sense for any customer deployment. I had my doubts.

#284 Updated by Greg Shah 8 months ago

If this is the default login for FWD, why do we need to add the logo and favicon to the application jar? I'd prefer if the default was available from the p2j.jar just like the rest of our core web resources.

I wasn't sure if it makes sense for any customer deployment. I had my doubts.

The existing login is already hard coded and it does not look as good as your new approach. The new approach is significantly better, I think. Why not include it in all cases and allow the override when present in the application jar?

#285 Updated by Galya B 8 months ago

Default favicon and logo added to the main project in r14761.

#286 Updated by Galya B 8 months ago

Greg Shah wrote:

Let's also make sure that the new login and the error message processing is honoring our I18N translations approach.

I reworked quite heavily the error messages (WebDriverHandler and SpawnErrorCodes). The login page will show only two messages from now on: Invalid username or password. and An unexpected error occurred. Please contact the server administrator! (Both found in WebDriverHandler.ClientFacingResultCode). I'm not sure what translation mechanisms there are in the project. Maybe a hint?

#287 Updated by Sergey Ivanovskiy 8 months ago

Greg Shah wrote:

Galya B wrote:

r14759 an updated default login screen (html + css). Desktop settings (bgr attributes from directory) removed.

The default login is kept simple, still using browser styling for select and some input attrs. On landscape with mobile overflow:hidden allows scrolling. I tried to keep the new html + css as lean/clean as possible.

I like the look and feel of the default. It is nice and clean.

Sergey: Please review the changes.

Greg, Galya what should be reviewed? Where are the source code?

#288 Updated by Greg Shah 8 months ago

Branch 3931a is where the changes are. Please focus on the web client login and admin console changes. Constantin is reviewing the core session/security changes.

#289 Updated by Greg Shah 8 months ago

Sergey: Please point Galya to some details for how we handle the I18N support in the JS side of the web client.

#290 Updated by Sergey Ivanovskiy 8 months ago

Greg Shah wrote:

Sergey: Please point Galya to some details for how we handle the I18N support in the JS side of the web client.

In terms of 3931a there exist WebResourceHandler.getGetTextJS

   /**
    * Gets gettext.js JavaScript module handler.
    * 
    * @param    basePath
    *           The parent context path. May be empty.
    *           
    * @return   The gettext.js JavaScript module handler.
    */
   public static Handler getGetTextJS(String basePath)
   {
      return new WebResourceHandler(basePath, "/gettext.js", "gettext.js-1.2.0");
   }

and EmbeddedWebServerImpl.getTranslations
   /**
    * Returns the supplier of a raw (unescaped) json string that holds all available translations
    * for the given language. If the language is not given or its translation resource does not exist, then
    * the default resource is used.
    * 
    * @param    lang
    *           The language identifier
    * @param    cachedLanguages
    *           The set of cached languages
    * @param    defaultValue
    *           The default value if i18n resources is not found for the given language
    * 
    * @return   The supplier of the language identifier and I18N resources for the given language.
    */
   public static Supplier<Entry<String, String>> getTranslations(String lang,
                                                                 Set<String> cachedLanguages,
                                                                 String defaultValue)
   {
      return I18N.getTranslations(lang, cachedLanguages, defaultValue);
   }

The first one returns web handler for /gettext.js requests from the client and the last one gets translation for the given language with external language translation cache so that these resources are delivered only once for the client.

The js web client loads gettext.js library and setups language translations.

      // initialize gettext.js
      i18n = window.i18n();

      Object.defineProperty(
            p2j,
            "i18n",
            {
               get: function ()
               {
                  return i18n;
               }
            });
      var lang = cfg["lang"];
      if (lang)
      {
         p2j.addTranslations(lang, cfg["translations"]);
      }

The examples of the usages of this library can be found in src/com/goldencode/p2j/ui/client/driver/web/res/p2j.js
loadingDialog.setTitle(i18n.gettext("Loading..."));

for a simple key that needs to be translated.

It needs to load translation resources and i18n gettext js library for login pages in order to add i18n support for them.

#291 Updated by Sergey Ivanovskiy 8 months ago

and translation resources are defined by special get text js json format. Please look at this file
src/com/goldencode/p2j/ui/client/driver/web/tr/WebClient_nl_NL.json.

#292 Updated by Galya B 8 months ago

Sergey, a few pointers about what's going on in 3931a:
  • com.goldencode.p2j.main.Webandler was renamed to WebDriverHandler to not be confused with com.goldencode.p2j.oo.web.WebHandler. WebDriverHandler is now a base class of type org.eclipse.jetty.server.handler.HandlerCollection for VirtualDesktopWebHandler and EmbeddedWebHandler. All these classes are completely reworked.
  • Introducing "SSO" (single login screen for in-app users that should replace OS login + 4GL login) is the main focus of this task. Embedded will work only with SSO enabled (auth-mode/ssoplugin provided), while virtual desktop retains its old flow as an alternative. Example configs for SSO:
          <node class="container" name="config">
            <node class="authMode" name="auth-mode">
              <node-attribute name="ssoplugin" value="com.goldencode.p2j.security.SampleSsoAuthenticator"/>
              <node-attribute name="ssopluginoption" value="bogus"/>
            </node>
          </node>
          <node class="container" name="accounts">
            <node class="container" name="osusers">
              <node class="osuser" name="[OS-USER]"/>
            </node>
            <node class="container" name="users">
              <node class="user" name="bogus">
                <node-attribute name="osuser" value="[OS-USER]"/>
              </node>
            </node>
          </node>
    
  • The webClient configs in directory are merged with embeddedWebApp and some attributes are removed (more details in #3931-274):
            <node class="container" name="webClient">
              <node class="boolean" name="virtualDesktop">
                <node-attribute name="value" value="TRUE"/>
              </node>
              <node class="boolean" name="embedded">
                <node-attribute name="value" value="FALSE"/>
              </node>
              <node class="boolean" name="embeddedRemote">
                <node-attribute name="value" value="FALSE"/>
              </node>
              <node class="string" name="cfgOverrides">
                <node-attribute name="value" value="client:cmd-line-option:startup-procedure=galya/sess.p"/>
              </node>
              <node class="boolean" name="override">
                <node-attribute name="value" value="TRUE"/>
              </node>
              <node class="string" name="defaultOsUser">
                <node-attribute name="value" value="gbb"/>
              </node>
              <node class="string" name="loginPageTitle">
                <node-attribute name="value" value="FWD Login Page"/>
              </node>
              <node class="string" name="logoutPageTitle">
                <node-attribute name="value" value="FWD Logout Page"/>
              </node>
            </node>
    
  • I managed to remove CORS. Changes in supported http methods and js.
I think Constantin will review most of the above-mentioned changes. Now what's more in tune with Greg's comment #3931-288: I added the changes for #7067 and #4571 here as well.
  • The login screen is now served in iframe to:
    • allow full customization for users with complex login flows like in #4571 or demanding designers (possibly #7067);
    • preserve the origin and allow us to add some js in the wrapper container to have different enhancements in the future (persistent client storage, fingerprints, etc).
  • To get rid of CORS the iframe uses srcdoc instead of src, making the iframe work under the same url as the container, basically making the iframe container invisible. On GET /gui, GET /embedded the resource web_landing.html is served with the resource web_partial_login.html loaded in a template, that is used to fill in the iframe srcdoc.
  • There is a logout page for virtual desktop with SSO. Otherwise the auto-login feature leads to indefinite loop. Stopping the auto-logging manually can be done by removing the cookie SampleSsoAuthenticator.
  • The login / logout page html / css / js has been updated or it's new. Also there are some related changes in some web driver js files.
  • All web resources (html, css, logo, favicon) can be replaced in the client project by adding a root dir webres and including it in the build script as shown in #3931-276. The WebDriverHandler.serveStatic and related methods usually first look for the resource in the custom package webres/ dir and then in p2j main. So #7067 can be fulfilled in multiple ways by the customer by:
    • providing a completely new login page by replacing web_partial_login.html with embedded css.
    • providing a completely new login page by replacing web_partial_login.html and also replacing styles.css.
    • keeping our reworked html and providing only styles.css, which will completely replace our css. I believe this is the superior alternative over overriding the base css. It's copy paste easy to get the base css in the new file and will solve multiple potential issues (like the need of using !important and bypasing constraints in the layout).
  • The admin UI now supports adding OS user to the FWD user account as well as managing a list of osusers and their password in a new submenu. Constantin reviewed the functionality and asked for some improvements in #3931-278, but I think Greg has a review of the code in mind for you.

#293 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

For consistency; you can have:

def var h as handle.
h = SESSION.
message h:auth-blob.

I've tested it and it works. I've followed other working examples, so it should be fine.

#294 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

  • AdminServiceImpl.setOsUser - if plainPassword is null/empty, don't we need to reset password?

Right, I thought it's null coming directly from the UI form as plainPassword. Fixing.

#295 Updated by Galya B 8 months ago

Constantin, I think No Password Protection is not working properly for FWD accounts as well, can you confirm? It's not deleting the pass even with protected being FALSE.

#296 Updated by Sergey Ivanovskiy 8 months ago

Galya, please help to understand what are advantages of renaming webClient/virtualDesktopEnabled -> webClient/virtualDesktop? What is the value of this setting?

#297 Updated by Sergey Ivanovskiy 8 months ago

I would prefer to use different tasks for SSO plugin and for login pages because these two tasks should be completely independent.

#298 Updated by Sergey Ivanovskiy 8 months ago

The code part <script/> of web_landing.html template does not use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode.
Is it done intentionally? What is the idea?

#299 Updated by Galya B 8 months ago

Sergey,

1. renaming of virtualDesktopEnabled was for consistency. The whole "enable" drivers thing was very messy. There was webClient/enabled, webClient/virtualDesktopEnabled, embeddedWebApp/enabled, webClient/embedded. The last two were interfering with each other, one was registering the web handlers, the other was overriding the client configs, so basically the server can serve both types of handlers, but embedded was always overriding the configs, when virtualDesktop and embedded were enabled. I looked into all of these and got them down to only two - webClient/virtualDesktop and webClient/embedded. Since webClient/embedded was already existing as an option for enabling, I thought I'll have the privilege to decide which one of both styles of naming stays. virtualDesktopEnabled is pretty ugly, so it went to the trash. I also added webClient/embeddedRemote because there is no reason to allow direct server access to launching, if it's not supposed to be used. Greg still considers the complete removal of the feature, so it will be easier this way.

2. I would love to have daily merges to trunk like I'm used to and then each task can be split into multiple independent ones with quick turnover. The thing is I need the base created in 3931a to easily fulfill #7067. All the tasks in my TODO list are related, so I don't really have the choice to work on something else, while the review of the first task is done. And it doesn't make sense to work in parallel on two related tasks that can greatly benefit from each other. I understand the review can get difficult, but if we define the scope, which I'm open to help with further, it may work.

3. I've missed to add it to the new script in web_landing.html . Fixed in r14764.

#300 Updated by Galya B 8 months ago

Constantin, here is the new look of the OS Users Management page. Please note that the second table (Associated FWD Users) will not be functional like the one on the Users page. OS users will be mapped to FWD users only in the FWD users' add/edit dialog. FWD users : groups is many : many, while FWD users : OS users is many : one and it will get messy if that single mapping can be changed in a different place.

Up in r14765.

#301 Updated by Galya B 8 months ago

I'm not sure how translation should work on the login page. gettext.js uses the language set by the 4GL session according to the JavaDoc.

In p2j.js:

var lang = cfg["lang"];

In WebClientBuilderOptions:

options.put("lang", EnvironmentOps.getCurrentLanguageString());

In EnvironmentOps:

   /**
    * Retrieves the string value of the session-specific current language.
    *
    * @return   The name of the current language.
    */
   public static String getCurrentLanguageString()
   {
      return work.obtain().currentLanguage;
   }

On the login page there is still no session, where is the lang value going to come from?

We can have a dropdown for selecting from all supported languages. Not sure if this is an option now.

#302 Updated by Sergey Ivanovskiy 8 months ago

If there is a requirement to internationalize login pages, then http request object gives the user client preferred languages and having translation resources for login pages for server accepted languages it is possible to define the preferred accepted locale and corresponding translation resources.

#303 Updated by Galya B 8 months ago

There is a regression related to #4938 (4938b merged in r14688).

ClientCore.processClientParams lost:

cp.startupProc  = config.getString("client", "cmd-line-option", "startup-procedure", null);

Embedded (and in 3931a VD as well) sets that config programmatically. It's not set in the directory in the standard process/user/clientConfig cfgOverrides, so now it's not sent to the server any more.

If customers with embedded are not going to be updated with trunk, I can fix it here. StandardServer.standardEntry will first check ClientParameters for startupProcedure and then StartupParameters.

#304 Updated by Galya B 8 months ago

Sergey, as far as I understand at the moment there is no config for languages outside of a 4GL app? I mean there is no directory config what languages are supported, what is the default one and their translations? All of this at the moment comes from the customer app?

#305 Updated by Greg Shah 8 months ago

If customers with embedded are not going to be updated with trunk, I can fix it here.

Yes, we can fix it here.

#306 Updated by Constantin Asofiei 8 months ago

Continue for the review of 3931a rev 14758 on top of trunk rev 14707.

3. SSO and embedded refactoring
  • LeafSessionManager, p2j.remote.js - no actual change, please revert the files to their trunk version
  • all new directory/bootstrap config options need to be documented (emphasize what has been renamed and what we need to migrate existing customers to these new values)
  • hand-editing of the directory can end up with an OS user set at the FWD user, but not registered as 'osusers'. Do not trust and validate the OS user when calculated from the FWD user - this is already done I think, but please double-check.
  • WebDriverHandler
    • there are changes lost from rebase in WebHandler.java (see H019 by DDF).
    • specify in the history entry that it was renamed
    • handle - can we make the mime type code for images more generic?
  • VirtualDesktopWebHandler
    • handle - add a break; for the default: branch in the switch (method)
  • window.location.href vs window.location.replace - replace prohibits back navigation (as the URL change is not recorded in history). Do we want to use href instead of replace?
  • the virtual desktop uses now an iframe; customers have automated testing using Selenium and Sikuli - we need to coordinate with them, how this affects this automated testing.
  • why is this condition in p2j.emain.js?

xmlHttp.responseText && xmlHttp.responseText.length < 50

  • EmbeddedWebHandler renamed from EmbeddedWebAppHandler - please fix the Module name at the top and specify in the history entry that it was renamed
  • web_partial_login.html - add the copyright notice
  • translations for all texts in the .js code and in the web handler Java code (for login, logout, page title, etc)
  • the FWD reverse proxy support needs to be re-tested
  • protect the handle(String target, Request...) code in a try/catch(Throwable t), otherwise any uncaught runtime exception will leak to the UI.
  • trusted (no password) and remote (for embedded) launch is broken.
    • in spawnWorker, the assignContext will set the context for the 'qtp' Jetty thread using the FWD user for the very first request in such mode - and this thread will remain with this context, for each and every future request! This is not OK.
    • with this approach, embeddedWebApp configs in the directory (for the FWD process account used by the thread doing the spawn) is no longer used; and, this means that any FWD user account which will be returned by SSO must have 'trusted spawner rights'.
4. SecurityManager refactoring
  • SecurityManager is used from customer applications, like #5731. You need to create a separate task in the customer's project, fix the issues from refactoring, test it, and coordinate with Greg/Sergey when and where to commit it (probably at the same time 3931a gets into trunk).
  • SecurityManagerAuthenticator
    • remove initSecurityManager and pass the instance at the constructor; this class can't exist without the SecurityManager instance anyway.
  • AccountExtUtil (and ensure all standalone classes extracted from SecurityManager work the same):
    • the c'tor must be package-private
    • the instance can be accessed via a field as e.g. SecurityManager.getInstance().accountExtUtil
    • this is because the SecurityManager singleton instance can be reset (see SecurityManager.clearInstance()); and instances which save the SecurityManager instance must have the same lifetime as the SecurityManager instance.
  • SecurityUtil
    • checkCallerWorker - add a static INTERNAL_CLASS_NAMES set with all the class names which needs to be ignored, and use INTERNAL_CLASS_NAMES.contains(possibleClass).
  • javadoc missing:
    • AccountExtUtil constructor, sm field

#307 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

3. SSO and embedded refactoring
  • LeafSessionManager, p2j.remote.js - no actual change, please revert the files to their trunk version

OK

  • all new directory/bootstrap config options need to be documented (emphasize what has been renamed and what we need to migrate existing customers to these new values)

Yes, I did it in the notes in this task, but it will be migrated to a doc. Can I start a draft somewhere? I want to document how SSO works as well. I was thinking also that we need a page for changelog for FWD. Instead of documenting all merges and bugfixes, it can only list the changes that require migration.

  • hand-editing of the directory can end up with an OS user set at the FWD user, but not registered as 'osusers'. Do not trust and validate the OS user when calculated from the FWD user - this is already done I think, but please double-check.

Yes, this is supported.

  • WebDriverHandler
    • there are changes lost from rebase in WebHandler.java (see H019 by DDF).

This part is completely reworked, especially in the latest revision, so it's not applicable any more. Also I think there was an issue with H019 - having isGui as an instance field for the single instance of the handler could be an issue. Enabling the driver, allows both /gui and /chui to be served at the same time, so the isGui flag should be derived from the request each time.

  • specify in the history entry that it was renamed

OK

  • handle - can we make the mime type code for images more generic?

Do you still review r14653? If yes, can you be more specific where was that image handling.

  • VirtualDesktopWebHandler
    • handle - add a break; for the default: branch in the switch (method)

OK

  • window.location.href vs window.location.replace - replace prohibits back navigation (as the URL change is not recorded in history). Do we want to use href instead of replace?

I haven't considered that aspect. At the moment the only two paths supported are login and logout pages, so it's not so important like before, but I'll revert it.

  • the virtual desktop uses now an iframe; customers have automated testing using Selenium and Sikuli - we need to coordinate with them, how this affects this automated testing.

I think they will need to add only one line of code for their tests to continue working: const window = document.querySelector("#main-iframe").contentWindow;

  • why is this condition in p2j.emain.js?

xmlHttp.responseText && xmlHttp.responseText.length < 50

To hide runtime exceptions (when jetty returns the html for a whole page).

  • EmbeddedWebHandler renamed from EmbeddedWebAppHandler - please fix the Module name at the top and specify in the history entry that it was renamed

The module name is fixed in latest revisions. The history entry will be added now.

  • web_partial_login.html - add the copyright notice

I wasn't sure we need that. The landing page already has copyright and then the login is served only in the iframe and then we have the same twice.

  • translations for all texts in the .js code and in the web handler Java code (for login, logout, page title, etc)

That's tricky. Please check my questions and advise: #3931-301, #3931-304.

  • the FWD reverse proxy support needs to be re-tested

I have no idea how that's configured. Any reference?

  • protect the handle(String target, Request...) code in a try/catch(Throwable t), otherwise any uncaught runtime exception will leak to the UI.

It will be returned, but not shown on the page - related to your question about xmlHttp.responseText.length < 50. I'll think about some type of error handler. I don't like the try / catch block for runtime exceptions. You may remember it caught me off guard once already.

  • trusted (no password) and remote (for embedded) launch is broken.
    • in spawnWorker, the assignContext will set the context for the 'qtp' Jetty thread using the FWD user for the very first request in such mode - and this thread will remain with this context, for each and every future request! This is not OK.

Are you sure? I was pretty convinced every request comes on its own new jetty Thread.

  • with this approach, embeddedWebApp configs in the directory (for the FWD process account used by the thread doing the spawn) is no longer used; and, this means that any FWD user account which will be returned by SSO must have 'trusted spawner rights'.

How so? If the associated OS user has password, why trusted?

#308 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

4. SecurityManager refactoring
  • SecurityManager is used from customer applications, like #5731. You need to create a separate task in the customer's project, fix the issues from refactoring, test it, and coordinate with Greg/Sergey when and where to commit it (probably at the same time 3931a gets into trunk).

Oh, I wouldn't have touched it if I knew... yikes.
Please let me know when and where to start working on it.
I think we should first complete the whole review.

  • SecurityManagerAuthenticator
    • remove initSecurityManager and pass the instance at the constructor; this class can't exist without the SecurityManager instance anyway.

Unfortunately that wacky initSecurityManager serves a purpose. The authenticator is instantiated by reflection only, so it needs a no-arg constructor.

  • AccountExtUtil (and ensure all standalone classes extracted from SecurityManager work the same):

I tested some, but AccountExtUtil methods are actually not used anywhere in the whole FWD codebase. 800 lines of code that are not used... I don't know why that is.
A note: The extracted classes are in AccountExtUtil, LegacyWebSecurityManager, SecurityManagerAuthenticator, SecurityUtil.java.

  • the c'tor must be package-private
  • the instance can be accessed via a field as e.g. SecurityManager.getInstance().accountExtUtil

I was hoping one sunny day all of these fields will be moved to independent classes, so it's best to move away from this approach. It's just for easier refactoring at the moment. And again AccountExtUtil methods are not used anywhere.
Also SecurityManager instantiating methods are public, so the constructors of the extracted classes should also be public.

  • this is because the SecurityManager singleton instance can be reset (see SecurityManager.clearInstance()); and instances which save the SecurityManager instance must have the same lifetime as the SecurityManager instance.

I get emotional. I'll need time to collect myself...

  • SecurityUtil
    • checkCallerWorker - add a static INTERNAL_CLASS_NAMES set with all the class names which needs to be ignored, and use INTERNAL_CLASS_NAMES.contains(possibleClass).

OK.

  • javadoc missing:
    • AccountExtUtil constructor, sm field

OK.

#309 Updated by Constantin Asofiei 8 months ago

Galya B wrote:

This part is completely reworked, especially in the latest revision, so it's not applicable any more. Also I think there was an issue with H019 - having isGui as an instance field for the single instance of the handler could be an issue. Enabling the driver, allows both /gui and /chui to be served at the same time, so the isGui flag should be derived from the request each time.

Thanks, I understand now. But please keep the history entry.

  • handle - can we make the mime type code for images more generic?

Do you still review r14653? If yes, can you be more specific where was that image handling.

I haven't reviewed further revisions yet. I mean this code:

         String fileName = target.replaceFirst("/" + PKG_RES_DIR, "");
         String mimeType = fileName.endsWith(".png") ?
            "image/png" : fileName.endsWith(".svg") ?
            "image/svg+xml" : fileName.endsWith(".ico") ?
            "image/x-icon" : "text/plain";

  • why is this condition in p2j.emain.js?

xmlHttp.responseText && xmlHttp.responseText.length < 50

To hide runtime exceptions (when jetty returns the html for a whole page).

As you mentioned in another place, we need an error handler.

  • translations for all texts in the .js code and in the web handler Java code (for login, logout, page title, etc)

That's tricky. Please check my questions and advise: #3931-301, #3931-304.

Will do.

  • the FWD reverse proxy support needs to be re-tested

I have no idea how that's configured. Any reference?

Sergey, can you help with this testing (or how to setup this)?

  • trusted (no password) and remote (for embedded) launch is broken.
    • in spawnWorker, the assignContext will set the context for the 'qtp' Jetty thread using the FWD user for the very first request in such mode - and this thread will remain with this context, for each and every future request! This is not OK.

Are you sure? I was pretty convinced every request comes on its own new jetty Thread.

Jetty threads are pooled and can be re-used.

  • with this approach, embeddedWebApp configs in the directory (for the FWD process account used by the thread doing the spawn) is no longer used; and, this means that any FWD user account which will be returned by SSO must have 'trusted spawner rights'.

How so? If the associated OS user has password, why trusted?

For Linux, it must work without a OS password specified - this is the 'trusted' mode.

Please let me know when and where to start working on it.
I think we should first complete the whole review.

Yes, when we are close to finish 3931a; it shouldn't be difficult the refactoring.

  • SecurityManagerAuthenticator
    • remove initSecurityManager and pass the instance at the constructor; this class can't exist without the SecurityManager instance anyway.

Unfortunately that wacky initSecurityManager serves a purpose. The authenticator is instantiated by reflection only, so it needs a no-arg constructor.

Hm... the SecurityManager.smAuthenticator is not created by reflection. OTOH, I think is enough to make the constructor package-private so that access tot hem is only via instance fields at SecurityManager. When SecurityManager gets reset, these fields will be re-initialized anyway when a new SecurityManager instance is created; if someone caches these fields, it was already possible with the SecurityManager.getInstance, anyway.

  • AccountExtUtil (and ensure all standalone classes extracted from SecurityManager work the same):

I tested some, but AccountExtUtil methods are actually not used anywhere in the whole FWD codebase. 800 lines of code that are not used... I don't know why that is.

This is used by the #5731 app.

Also SecurityManager instantiating methods are public, so the constructors of the extracted classes should also be public.

See above: I just want to make sure that any code using these extracted classes can use them only via SecurityManager, so the correct instance is used; if someone caches it, then that is already possible by caching SecurityManager.getInstance, anyway.

#310 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

Unfortunately that wacky initSecurityManager serves a purpose. The authenticator is instantiated by reflection only, so it needs a no-arg constructor.

Hm... the SecurityManager.smAuthenticator is not created by reflection.

Well... right. It implements the interface Authenticator and at some point I was thinking about the use with directory auth-mode/plugin, but it actually is a very different case and it seems it's not expected ever to replace that plugin in configs.

OTOH, I think is enough to make the constructor package-private so that access tot hem is only via instance fields at SecurityManager. When SecurityManager gets reset, these fields will be re-initialized anyway when a new SecurityManager instance is created; if someone caches these fields, it was already possible with the SecurityManager.getInstance, anyway.

I see. This is a good solution.

#311 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

  • handle - can we make the mime type code for images more generic?

Do you still review r14653? If yes, can you be more specific where was that image handling.

I haven't reviewed further revisions yet. I mean this code:
[...]

You've started reviewing r14653, but this code is in one of the latest revisions. So probably you've updated unintentionally. Actually I was going to ask you to update anyways. There are no huge changes overall, but the child handlers have much cleaner code.
I thought about it, but couldn't get a more generic solution. If the images are not the exact matching mime type, they are not displayed properly in some browsers. At the same I couldn't find a method to resolve them automatically. And also the types are not one to one match with the file ext, so it can't be derived easily. If you can think of another solution, please let me know.

  • trusted (no password) and remote (for embedded) launch is broken.
    • in spawnWorker, the assignContext will set the context for the 'qtp' Jetty thread using the FWD user for the very first request in such mode - and this thread will remain with this context, for each and every future request! This is not OK.

Are you sure? I was pretty convinced every request comes on its own new jetty Thread.

Jetty threads are pooled and can be re-used.

All right, I'll just set the context anew every time it's not a remote call.

  • with this approach, embeddedWebApp configs in the directory (for the FWD process account used by the thread doing the spawn) is no longer used; and, this means that any FWD user account which will be returned by SSO must have 'trusted spawner rights'.

How so? If the associated OS user has password, why trusted?

For Linux, it must work without a OS password specified - this is the 'trusted' mode.

Before there was the certificate associated with an alias - fwd account, but the os user came from webClient/defaultAccount and was without password. So embedded still triggered the check for trustedspawner in WebHandler.spawn.
Configuring trustedspawner subjects for all FWD users working with passwordless OS users is indeed troublesome, so I was asking before do we really need it. It seems to me like a complication that doesn't bring much benefit.

#312 Updated by Galya B 8 months ago

Btw the mime types look neater in the latest revision. I've put them in a map.

#313 Updated by Galya B 8 months ago

Greg, I remember you wanted remotelaunchoption to be checked for all web spawn requests, instead of just those from rpc, but it can't be done for VD without SSO, because the FWD user can't be identified at that time to create the security context for the check. Instead the FWD user gets known at the time the client connects to the server and the auth-mode/plugin does its thing. The authenticator can be GuestAccess with FWD user as an option, but it can be something else as well.

So for now remotelaunchoption is supported only for rpc calls. I can also add it to SSO cases, but it will introduce inconsistency.

#314 Updated by Galya B 8 months ago

r14767 Review part II improvements applied.

#315 Updated by Galya B 8 months ago

Can we remove reverse proxies for customers?

The /gui, /chui, /embedded port is configurable and if that is 443 it won't show up in the address bar. While with the iframe now the other ports / broker addresses are hidden from the customer.

I looked into Reverse Proxy and the related configs and I don't see a reason to keep them.

If reverse proxy are removed, this will save the day... I mean my code...

By the way this task resolves also #7430 (found while looking for reversed proxy).

#316 Updated by Greg Shah 8 months ago

I don't see how we can eliminate the reverse proxy support. Some customers require that only the server's port 443 is exposed on the Internet, thus requiring reverse proxy to work with the dedicated ports of the FWD clients. Other customers are more flexible with the port access and can avoid the overhead and complexity that comes with reverse proxy. We must support both.

#317 Updated by Galya B 8 months ago

The variables in the address are too many and both the Java servlets and the js code have difficulties to imagine where to find the login / logout / referrer pages. The server port is one, the external port is different. Different contexts are served by the same code, both back-end and front-end... Auto-login adds even more flavor. Messy, messy...

The easy way out was if directory knew the external address for login / logout.

I'll have to rework something more drastically...

#318 Updated by Greg Shah 8 months ago

The easy way out was if directory knew the external address for login / logout.

How is this different from the server knowing the address(es) on which it is listening?

#319 Updated by Galya B 8 months ago

I'm speaking about redirections and links... On app end JS redirects to the logout page in SSO. The taskbar menu should have a link to the login page for #4856. Referrer is taken from the back-end request and fed to the js client used in a few places, but it's not the same with auto-login. Anyways, I'll figure it out.

P.S. This all is a bit more complicated taken into consideration the iframe.

#320 Updated by Constantin Asofiei 8 months ago

Galya B wrote:

All right, I'll just set the context anew every time it's not a remote call.

The context needs to be cleared once the remote call finishes, on the qtp/Jetty thread.

Greg: regarding the trusted mode; the above can work (set/clear the FWD context on each call), but this will require the FWD user to have 'trusted spawner rights', if no OS password is set; I'm thinking we can automate this in the FWD Admin Console, but I'm not sure how easy will be - we would need to identify this ACL - what if there are multiple?, and automatically add/remove the FWD user. More, if the directory is edited manually (via a text editor or via programmatic FWD APIs, by the application itself), then things will get tricky. At least, we need a proper error message being shown (something like Not enough permissions to perform the login on the Web page and a more verbose message in the FWD server log, like FWD user is not part of the trusted spawner ACL, it needs to be added); but this will require a FWD Admin Console screen to resolve it without restarting the FWD server.

#321 Updated by Greg Shah 8 months ago

Constantin Asofiei wrote:

Galya B wrote:

All right, I'll just set the context anew every time it's not a remote call.

The context needs to be cleared once the remote call finishes, on the qtp/Jetty thread.

Greg: regarding the trusted mode; the above can work (set/clear the FWD context on each call), but this will require the FWD user to have 'trusted spawner rights', if no OS password is set; I'm thinking we can automate this in the FWD Admin Console, but I'm not sure how easy will be - we would need to identify this ACL - what if there are multiple?, and automatically add/remove the FWD user. More, if the directory is edited manually (via a text editor or via programmatic FWD APIs, by the application itself), then things will get tricky. At least, we need a proper error message being shown (something like Not enough permissions to perform the login on the Web page and a more verbose message in the FWD server log, like FWD user is not part of the trusted spawner ACL, it needs to be added); but this will require a FWD Admin Console screen to resolve it without restarting the FWD server.

This sounds like a mess.

Let's back up a little. The point of trusted spawning mode is to allow spawning to occur for an OS user without having to encode the user's password in the directory. For Linux deploymnets, that can save a huge amount of admin overhead. I don't care so much about the specific trusted spawner mechanism that we had previously written. That code was convenient to put in place on top of what I consider to be a sub-optimal separated server approach for embedded mode. When running in an enviornment where there is no existing session, then I think we should handle this same requirement a different way. Sticking with the security manager resource approach may not work.

#322 Updated by Greg Shah 8 months ago

Sergey, as far as I understand at the moment there is no config for languages outside of a 4GL app? I mean there is no directory config what languages are supported, what is the default one and their translations? All of this at the moment comes from the customer app?

Correct. We could put settings in for session:current-language and/or other application-specific I18N settings.

#323 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

At least, we need a proper error message being shown (something like Not enough permissions to perform the login on the Web page and a more verbose message in the FWD server log, like FWD user is not part of the trusted spawner ACL, it needs to be added); but this will require a FWD Admin Console screen to resolve it without restarting the FWD server.

It's convenient when end-users tell admins what exactly went wrong by quoting the error message, but at the same time all of these are admin misconfigurations. I'm not sure if FWD end-users are different from any other, but in a standard web application, you don't want them to know what you've messed up. For better user experience you just want them to know if it's their fault (wrong credentials) or your fault (something went wrong). That's why I took this approach and left only two client facing messages, while the server will log all issues. That being said, most logs are on level FINE, because of web heavy app with many users it will get verbose. But important misconfigurations like missing SsoAuthenticator or missing os user are on INFO and higher.

What do you think of this approach?

#324 Updated by Galya B 8 months ago

Greg Shah wrote:

When running in an enviornment where there is no existing session, then I think we should handle this same requirement a different way. Sticking with the security manager resource approach may not work.

I got puzzled by this mechanism in the context of web spawn from the very beginning, because it doesn't make sense. I'm not sure what we're trying to achieve. What are the requirements from security perspective? What do we want to prevent by checking if a FWD user have the right to spawn under OS user without password? Or anything related?

I would probably just remove this plugin.

#325 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

Galya B wrote:

All right, I'll just set the context anew every time it's not a remote call.

The context needs to be cleared once the remote call finishes, on the qtp/Jetty thread.

Very good point.

#326 Updated by Greg Shah 8 months ago

It's convenient when end-users tell admins what exactly went wrong by quoting the error message, but at the same time all of these are admin misconfigurations. I'm not sure if FWD end-users are different from any other, but in a standard web application, you don't want them to know what you've messed up.

We aren't here to spare the feelings of some admins. :) We want the system to be very easy to support/diagnose problems. We have a long way to go to achieve that. It won't help by reducing the level of detail provided on the failures. Each separate failure should retain its own return value and text so that it is easy to trace back to the exact failure without any extra steps.

For better user experience you just want them to know if it's their fault (wrong credentials) or your fault (something went wrong).

Users are going to be annoyed either way. Trying to obscure it doesn't really help. It just makes it harder to find the root cause, which costs more time than the old approach. Please restore the previous level of detail.

#327 Updated by Greg Shah 8 months ago

When running in an enviornment where there is no existing session, then I think we should handle this same requirement a different way. Sticking with the security manager resource approach may not work.

I got puzzled by this mechanism in the context of web spawn from the very beginning, because it doesn't make sense. I'm not sure what we're trying to achieve. What are the requirements from security perspective? What do we want to prevent by checking if a FWD user have the right to spawn under OS user without password? Or anything related?

At the time it was a mechanism to allow security checking of spawning rights for a remote system that was sending spawn requests outside of our control.

In a world where that remote system is no longer there, I don't think it makes sense. The SSO authentication itself is now securing the spawn.

I would probably just remove this plugin.

Yes, I think we should do that and avoid the complexities of keeping some kind of session around to process this.

Constantin: We only have that one original POC that is affected by this. It hasn't been run in years. I think we would even be able to run it in the new approach (hosted in the FWD server instead of remote). If that customer wanted to move ahead with a remote approach, we could provide an alternative API at that time. What do you think?

#328 Updated by Galya B 8 months ago

Greg Shah wrote:

Please restore the previous level of detail.

The number / type of errors in the web handlers went through the roof, the places where the errors were produced are many. There were chains of methods that had to move around errors. Some with more than one parameter. It became very scary. That was the original reason why I simplified it for the response. Many errors were originally NOT logged. I turned them all to logs and suddenly the code made sense. And of course I thought it's not the end-user's business what the admin had misconfigured. Most of the time the config will affect many users. Now all errors are properly logged.

Do we really need the chaos back?

#329 Updated by Galya B 8 months ago

Also translating the errors with Google translator may get funny for things like Cannot load dynamic libraries.. :)

#330 Updated by Greg Shah 8 months ago

Galya B wrote:

Greg Shah wrote:

Please restore the previous level of detail.

The number / type of errors in the web handlers went through the roof, the places where the errors were produced are many. There were chains of methods that had to move around errors. Some with more than one parameter. It became very scary. That was the original reason why I simplified it for the response. Many errors were originally NOT logged. I turned them all to logs and suddenly the code made sense. And of course I thought it's not the end-user's business what the admin had misconfigured. Most of the time the config will affect many users. Now all errors are properly logged.

Do we really need the chaos back?

I don't require chaos, but I do require the detailed errors to be returned back to the login panel. Is the problem that it is messy to pass this info all the way through the call stack? We can implement a kind of status class that can be passed down as deep as needed and can hold the resulting information so that the return values don't have to be manipulated.

#331 Updated by Galya B 8 months ago

And you want them translated or not?

#332 Updated by Greg Shah 8 months ago

Galya B wrote:

And you want them translated or not?

Yes. If there is any problem with the translations, our customers in those locales will assist.

#333 Updated by Galya B 8 months ago

Just mentioning on time... the whole confusion of the end user about messages about FWD user / account and OS user / account that are not actually their account will be fun.

#334 Updated by Greg Shah 8 months ago

Galya B wrote:

Just mentioning on time... the whole confusion of the end user about messages about FWD user / account and OS user / account that are not actually their account will be fun.

For small values of fun.

#335 Updated by Galya B 8 months ago

I myself had that issue multiple times of not understanding why the credentials are not accepted according to the message. Now it says OS user, but on the first glance it's still misleading, even for me. It will be a newly introduced situation, because there were only the OS credentials before on that screen.

I'm adding all the errors to the ui, but I just want to mention a superior and more reliable debugging approach. That is each login / spawn error has uuid in the logs and the user receives the generic msg and the uuid. Having the stacktrace, time, context coming with the logs makes it way easier to debug and most importantly reliable. Also we can separate all the web login attempt logs in a new file. It can be useful for debugging port allocation issues as well. It will be useful for quickly spotting misconfigurations of sso.

#336 Updated by Galya B 8 months ago

Not to mention (mentioning) this approach will eliminate the need to keep translations for 10+ errors.

#337 Updated by Galya B 8 months ago

A question - since the same error is supposed to be present in the logs, what language should it be in? If the login screen shows translated errors, I guess the logs should also log the same msg, although every other one will be in English.

#338 Updated by Galya B 8 months ago

One more to the topic: Translating parameterized messages should be done before the specifiers are substituted. I can't find a way / wiki on how to load translation resource bundle without session on the server, so this operation will have to be done on the web ui. But then it can't support Java string format specifiers. Also translating on the ui will mean logs with different content on the server (as already mentioned). Do we want the web end users to become the golden standard for debugging?

#339 Updated by Greg Shah 8 months ago

A question - since the same error is supposed to be present in the logs, what language should it be in? If the login screen shows translated errors, I guess the logs should also log the same msg, although every other one will be in English.

Yes

#340 Updated by Greg Shah 8 months ago

We have other tasks open to translate all other messages, so eventually all the messages will be translated.

#341 Updated by Galya B 8 months ago

Translations is the last sub-task I have before a final review. I need more specific pointers how to solve it with the current setup.

To have the same logs and error on the UI, we need server-side translations without client session. I don't find a way in the code or in the docs to achieve that at the moment. Am I missing something?

#342 Updated by Sergey Ivanovskiy 8 months ago

Galya B wrote:

One more to the topic: Translating parameterized messages should be done before the specifiers are substituted. I can't find a way / wiki on how to load translation resource bundle without session on the server, so this operation will have to be done on the web ui. But then it can't support Java string format specifiers. Also translating on the ui will mean logs with different content on the server (as already mentioned). Do we want the web end users to become the golden standard for debugging?

On the js web client side we use gettext jscript, but you need to do translations on the server side, correct? You can use gettext java or TranslationManager.applyLanguage, or just ResourceBundle to get the translated key and specify all keys and messages in properties files corresponding to the language. For an example, you can create login.properties, login_language_identifier.properties, where all keys and messages are collected and translated. Or you can associate a resource with java class that is responsible to provide login page to users. Hynek, could you advise how to use TranslationManager.applyLanguage for this task?

#343 Updated by Sergey Ivanovskiy 8 months ago

I guess that it needs to

long refid = TranslationManager.initReferent(JavaClassResponsibleToLoadLoginPage);
TranslationManage.applyLanguage(refid);
String translatedMessage = TranslationManager.translate(refid, message);

#344 Updated by Constantin Asofiei 8 months ago

Galya B wrote:

I'm adding all the errors to the ui, but I just want to mention a superior and more reliable debugging approach. That is each login / spawn error has uuid in the logs and the user receives the generic msg and the uuid.

I agree about the UUID - being present both at the login failure message and in the log. The big part here is that all log message related to the login failure must be marked with this UUID. Having the stacktrace, time, context coming with the logs makes it way easier to debug and most importantly reliable.

Also we can separate all the web login attempt logs in a new file. It can be useful for debugging port allocation issues as well. It will be useful for quickly spotting misconfigurations of sso.

I'm not sure about this. Remember the CONSOLE mode for CentralLogger? Clients really use it - they expect to track STDOUT/STDERR from the FWD Server process.

#345 Updated by Sergey Ivanovskiy 8 months ago

Sergey Ivanovskiy wrote:

I guess that it needs to

long refid = TranslationManager.initReferent(JavaClassResponsibleToLoadLoginPage);
TranslationManage.applyLanguage(refid);
String translatedMessage = TranslationManager.translate(refid, message);

No, you need a language, but the code above will use the current language. So I think that it needs to use ResourceBundle.getBundle(...) directly

#346 Updated by Galya B 8 months ago

Sergey Ivanovskiy wrote:

I guess that it needs to
[...]

I tried earlier to set it though EnvironmentOps and it threw an exception for remote object, but maybe using directly TranslationManager works. I also stumbled on what is JavaClassResponsibleToLoadLoginPage.

#347 Updated by Sergey Ivanovskiy 8 months ago

Galya B wrote:

Sergey Ivanovskiy wrote:

I guess that it needs to
[...]

I tried earlier to set it though EnvironmentOps and it threw an exception for remote object, but maybe using directly TranslationManager works. I also stumbled on what is JavaClassResponsibleToLoadLoginPage.

I think you should try ResourceBundle.getBundle(String baseName, Locale locale) to load required translations and resource bundles can be associated with VirtualDesktopWebHandler.class that is responsible to load login pages.

#348 Updated by Galya B 8 months ago

Sergey Ivanovskiy wrote:

I think you should try ResourceBundle.getBundle(String baseName, Locale locale) to load required translations and resource bundles can be associated with VirtualDesktopWebHandler.class that is responsible to load login pages.

If I understand correctly, TranslationManager will not be needed? Just go for the base Java approach?

#349 Updated by Galya B 8 months ago

Constantin Asofiei wrote:

Galya B wrote:

I'm adding all the errors to the ui, but I just want to mention a superior and more reliable debugging approach. That is each login / spawn error has uuid in the logs and the user receives the generic msg and the uuid.

I agree about the UUID - being present both at the login failure message and in the log. The big part here is that all log message related to the login failure must be marked with this UUID. Having the stacktrace, time, context coming with the logs makes it way easier to debug and most importantly reliable.

My idea was that all errors sent to the end-user will have the uuid and they will be logged with the same id. But the rest of the related logs won't have ids.

Currently this is logged for success:

23/09/14 18:29:32.811+0300 |    FINE | com.goldencode.p2j.main.WebDriverHandler | ThreadName:qtp754592847-75 | Attempting spawn for SSO authenticated app user inappuser.
23/09/14 18:29:32.812+0300 |    FINE | com.goldencode.p2j.main.WebDriverHandler | ThreadName:qtp754592847-75, Session:00000004, Thread:00000006, User:newuser | Attempting spawn under OS user gbb.
23/09/14 18:29:35.442+0300 |    INFO | com.goldencode.p2j.main.WebDriverHandler | ThreadName:qtp754592847-75, Session:00000004, Thread:00000006, User:newuser | Returns the redirect url https://localhost:7449/index.html?token=9b1ff43b454936ecda6448a0c3a36a32 as response to GUI client.

For failure:

23/09/14 18:32:27.191+0300 |    FINE | com.goldencode.p2j.main.WebDriverHandler | ThreadName:qtp754592847-38 | Attempting spawn for SSO authenticated app user sdf.
23/09/14 18:32:27.192+0300 | WARNING | com.goldencode.p2j.main.WebDriverHandler | ThreadName:qtp754592847-38 | Attempt to use defaultOsUser, but it's not defined [override true].

The last one, the WARNING is sent back to the ui. It can have uuid in the logs and in the ui. And by looking at the uuid, ThreadName and time, you can determine there was really an issue and what it is. My point is that the end user doesn't have to receive all weird messages for us to have reliable source of info.

Also we can separate all the web login attempt logs in a new file. It can be useful for debugging port allocation issues as well. It will be useful for quickly spotting misconfigurations of sso.

I'm not sure about this. Remember the CONSOLE mode for CentralLogger? Clients really use it - they expect to track STDOUT/STDERR from the FWD Server process.

My point here was that the above FINE logs will be usually filtered out by the logging configs. They can be enabled of course, but it may get verbose, that's why a different file is recommended. Also I really hope clients don't use the console handler. It substantially decreases performance. I thought I was adding it for us FWD devs.

#350 Updated by Sergey Ivanovskiy 8 months ago

Galya B wrote:

Sergey Ivanovskiy wrote:

I think you should try ResourceBundle.getBundle(String baseName, Locale locale) to load required translations and resource bundles can be associated with VirtualDesktopWebHandler.class that is responsible to load login pages.

If I understand correctly, TranslationManager will not be needed? Just go for the base Java approach?

I don not know what approach is better to follow. In any case an input locale and a resource for this locale are all that required for translations.

#351 Updated by Hynek Cihlar 8 months ago

Currently we don't have translation resolved for FWD runtime (beside Javascript client). TranslationManager addresses translation needs for the legacy code and so it is not suitable for the FWD runtime itself.

If there is no objection I will add proper support for string translation also for FWD runtime.

#352 Updated by Constantin Asofiei 8 months ago

Galya B wrote:

My idea was that all errors sent to the end-user will have the uuid and they will be logged with the same id. But the rest of the related logs won't have ids.

Correct, just for the user login messages, the UUID will be there.

The last one, the WARNING is sent back to the ui. It can have uuid in the logs and in the ui. And by looking at the uuid, ThreadName and time, you can determine there was really an issue and what it is.

The problem here is this: we need to log any and all relevant info, with the UUID, but not via FINE; think of problems when the app is deployed in production - we need to have all this always logged, not conditional on 'well, this is only on FINE level'. So: if a login failure happens because of a misconfiguration at the FWD server, all info needs to be logged always, hopefully relevant enough for an admin to determine what config is wrong.

#353 Updated by Greg Shah 8 months ago

If there is no objection I will add proper support for string translation also for FWD runtime.

Go ahead.

#354 Updated by Galya B 8 months ago

Hynek Cihlar wrote:

If there is no objection I will add proper support for string translation also for FWD runtime.

If you can explain your idea, I can implement it to unblock this task.

#355 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

Hynek Cihlar wrote:

If there is no objection I will add proper support for string translation also for FWD runtime.

If you can explain your idea, I can implement it to unblock this task.

I'm already partly done. I expect to have it ready in a couple of hours.

#356 Updated by Galya B 8 months ago

I didn't know you've started. I'm ready with the implementation in 3931a r14777. Can you please check if it's in check with what you have in mind. I think it's a very slick and clean solution.

#357 Updated by Galya B 8 months ago

WebDriverHandler error msg added to translation resources in r14778. Next I'll translate the login / logout pages and I'm done with the task.

#358 Updated by Galya B 8 months ago

It's done in r14780.

With this config the login can be translated:

    <node class="container" name="server">
      <node class="container" name="default">
        <node class="string" name="current-language">
          <node-attribute name="value" value="Dutch"/>
        </node>
      </node>
    </node>

#359 Updated by Galya B 8 months ago

Constantin, Sergey, Hynek, 3931a is ready for final review.

#360 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

I didn't know you've started. I'm ready with the implementation in 3931a r14777. Can you please check if it's in check with what you have in mind. I think it's a very slick and clean solution.

OK, no problem. I checked r14777 and below some points.

We want the translations to be based on GNU gettext. This is a de facto industry standard, it's simple to use, has great tooling support, and provides full set of translation features like plurals for example. We already use it for the legacy code translation needs and also in Javascript driver code.

Functionally the translation must adhere to the languages set in TransactionManager. TransactionManager already keeps the languages assigned either in the environment or assigned in the legacy code through CURRENT-LANGUAGE. CURRENT-LANGUAGE may be assigned multiple languages.

We want to use resource bundles (generated by the GNU gettext tooling). They generally perform better than property files and allow features like plural forms.

The actual translation method used in the code should be named so that it is very short and least obstructive. Also it should provide string interpolation.

We probably want to keep all translations for the entire FWD runtime in a single resource bundle. i.e. one language one bundle. I find it easier to maintain single bundle for such relatively small number of strings. On the other hand the translation tools do seem to handle multiple bundles pretty well so I'm open to discussion on this one. I'm afraid we may end up with many bundles each having only a couple of entries.

See the attached diff, it provides a possible gettext implementation for FWD runtime. I haven't tested it yet, I got some issue with running unit tests from my IDE.

#361 Updated by Galya B 8 months ago

CURRENT-LANGUAGE will be set by directory.xml. As far as I can see from any examples it is the full language name. How do you map it to the abbreviation of the language used in the resource file names?

#362 Updated by Hynek Cihlar 8 months ago

Hynek Cihlar wrote:

See the attached diff, it provides a possible gettext implementation for FWD runtime. I haven't tested it yet, I got some issue with running unit tests from my IDE.

I should point out that the attached diff only implements the server side support, for client side some state serialization will be required.

#363 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

CURRENT-LANGUAGE will be set by directory.xml. As far as I can see from any examples it is the full language name. How do you map it to the abbreviation of the language used in the resource file names?

Yes. CURRENT-LANGUAGE is an arbitrary string assigned by the legacy code, so some kind of project specific translation to ISO language ids will be required. AFAIK the current project that comes in play does use ISO language identifiers.

#364 Updated by Galya B 8 months ago

Hynek Cihlar wrote:

Galya B wrote:

CURRENT-LANGUAGE will be set by directory.xml. As far as I can see from any examples it is the full language name. How do you map it to the abbreviation of the language used in the resource file names?

See #3931-322. There should be a link between the language in the runtime and the one in the session. That's why I added the above directory config that is the full language name.

CURRENT-LANGUAGE may be assigned multiple languages.

It can't for the runtime, because there is no way to switch between them.

#365 Updated by Galya B 8 months ago

Hynek Cihlar wrote:

We want the translations to be based on GNU gettext. This is a de facto industry standard

Having an external tool not plugged into the automation doesn't sound very standard for modern programs. But it is for the project, I have to acknowledge that. If the same can be achieved without any additional tool or library, that I would call the standard. It's just that gettext is already used by the web driver's front-end. But in #3931 the front-end will be translated server-side.

They generally perform better than property files and allow features like plural forms.

That's because it has all constants already loaded in memory when the JVM starts. There is no free lunch.

The actual translation method used in the code should be named so that it is very short and least obstructive.

This is a C dogma I see a lot in this project. Self-documenting is a more recent one.

I find it easier to maintain single bundle for such relatively small number of strings.

The thing is that those "bundles" are sent to the client, while the new texts will be used only on the server. The rule is to avoid unnecessary bigger sized files.

At the end of the day the new code in TranslationManager.tr does the same as my implementation in RuntimeTranslationManager.translate: reading from java ResourceBundle and finding the value by a key. But I added a few neat features: it works with current-language full language value, also it loads the bundle beforehand instead of on translation attempt. Also it falls back to the default bundle or returns the key if nothing found, which is very important, since it's going to be used for important errors, something needs to be logged always.

Basically it looks like the same thing, but plugged in the legacy class. Here is my implementation.


package com.goldencode.p2j.translation;

import com.goldencode.p2j.util.*;
import com.goldencode.p2j.util.logging.*;
import com.goldencode.util.*;

import java.io.*;
import java.util.*;

/** Translation manager for the FWD runtime */
public class RuntimeTranslationManager
{
   /** Current language set by directory */
   public static final String CURRENT_LANGUAGE =
      Utils.getDirectoryNodeString(null, "current-language", null, false);

   /** Directory with translation resources */
   private static final String TRANSLATIONS_DIR =
      "/" + RuntimeTranslationManager.class.getPackage().getName().replace('.', '/') + "/";

   /** Logger */
   private static final CentralLogger LOG = CentralLogger.get(RuntimeTranslationManager.class);

   /** Current language translations bundle */
   private ResourceBundle translationsBundle;

   /** Default language translations bundle */
   private ResourceBundle defaultTranslationsBundle;

   /**
    * Public constructor for RuntimeTranslationManager.
    * 
    * @param    bundleName
    *           The name of the bundle.
    */
   public RuntimeTranslationManager(String bundleName)
   {
      if (StringHelper.hasContent(CURRENT_LANGUAGE))
      {
         try
         {
            InputStream inputStream = getClass().getResourceAsStream(
               TRANSLATIONS_DIR + bundleName + "_" + CURRENT_LANGUAGE + ".properties");
            translationsBundle = new PropertyResourceBundle(inputStream);
         }
         catch (IOException e)
         {
            LOG.warning("Can't load translations bundle " + bundleName + " for language " + CURRENT_LANGUAGE);
         }
      }

      try
      {
         InputStream inputStream = getClass().getResourceAsStream(
            TRANSLATIONS_DIR + bundleName + ".properties");
         defaultTranslationsBundle = new PropertyResourceBundle(inputStream);
      }
      catch (IOException e)
      {
         LOG.warning("Can't load default translations bundle " + bundleName);
      }
   }

   /**
    * Getter for {@link #CURRENT_LANGUAGE}
    *
    * @return   See above.
    */
   public static String getCurrentLanguage()
   {
      return CURRENT_LANGUAGE;
   }

   /**
    * Returns the translated text.
    * 
    * @param    text
    *           The text to translate.
    * 
    * @return   See above.
    */
   public String translate(String text)
   {
      String translatedText = null;

      if (translationsBundle != null)
      {
         try
         {
            translatedText = translationsBundle.getString(text);
         }
         catch (Throwable ignored)
         {
         }
      }

      if (defaultTranslationsBundle != null && translatedText == null)
      {
         try
         {
            translatedText = defaultTranslationsBundle.getString(text);
         }
         catch (Throwable ignored)
         {
         }
      }
      return translatedText == null ? text : translatedText;
   }

}

#366 Updated by Greg Shah 8 months ago

We want the translations to be based on GNU gettext. This is a de facto industry standard, it's simple to use, has great tooling support, and provides full set of translation features like plurals for example. We already use it for the legacy code translation needs and also in Javascript driver code.

Functionally the translation must adhere to the languages set in TransactionManager. TransactionManager already keeps the languages assigned either in the environment or assigned in the legacy code through CURRENT-LANGUAGE. CURRENT-LANGUAGE may be assigned multiple languages.

We want to use resource bundles (generated by the GNU gettext tooling). They generally perform better than property files and allow features like plural forms.

The actual translation method used in the code should be named so that it is very short and least obstructive. Also it should provide string interpolation.

We probably want to keep all translations for the entire FWD runtime in a single resource bundle. i.e. one language one bundle. I find it easier to maintain single bundle for such relatively small number of strings. On the other hand the translation tools do seem to handle multiple bundles pretty well so I'm open to discussion on this one. I'm afraid we may end up with many bundles each having only a couple of entries.

I agree with all of these design points. We definitely want the internal FWD translation support to be consistent/compatible/overlapping with the Translation Manager support.

I also agree with the single bundle approach. I don't think we have that many strings to translate, the majority of them will be error messages and there are only a couple thousand of those, max.

#367 Updated by Greg Shah 8 months ago

CURRENT-LANGUAGE will be set by directory.xml. As far as I can see from any examples it is the full language name. How do you map it to the abbreviation of the language used in the resource file names?

See #3931-322. There should be a link between the language in the runtime and the one in the session. That's why I added the above directory config that is the full language name.

CURRENT-LANGUAGE may be assigned multiple languages.

It can't for the runtime, because there is no way to switch between them.

What do you mean by this? SESSION:CURRENT-LANGUAGE is assignable and we react to it.

#368 Updated by Galya B 8 months ago

Greg Shah wrote:

What do you mean by this? SESSION:CURRENT-LANGUAGE is assignable and we react to it.

Greg, please, don't make me feel like an alien :) Maybe I have to start from zero and explain why this whole thing has nothing to do with the sessionless translations I'm doing. Or what about we do the translations in a separate task. This one is for SSO. We can further discuss the new feature and assign it to the best candidate.

#369 Updated by Greg Shah 8 months ago

Having an external tool not plugged into the automation doesn't sound very standard for modern programs.

We do support this gettext usage from our converted code. In other words, the 4GL Translation Manager approach is fully supported as part of our conversion.

Do you mean a different kind of automation?

That's because it has all constants already loaded in memory when the JVM starts. There is no free lunch.

I don't want to use a different approach for the Java code than for the legacy code.

At the end of the day the new code in TranslationManager.tr does the same as my implementation in RuntimeTranslationManager.translate: reading from java ResourceBundle and finding the value by a key. But I added a few neat features: it works with current-language full language value, also it loads the bundle beforehand instead of on translation attempt. Also it falls back to the default bundle or returns the key if nothing found, which is very important, since it's going to be used for important errors, something needs to be logged always.

Basically it looks like the same thing, but plugged in the legacy class. Here is my implementation.

[...]

Hynek will review the implementation. I do want to have common code instead of a separate translation manager for the Java code but the internal implementation is up to the two of you to discuss.

#370 Updated by Galya B 8 months ago

Greg Shah wrote:

Having an external tool not plugged into the automation doesn't sound very standard for modern programs.

We do support this gettext usage from our converted code. In other words, the 4GL Translation Manager approach is fully supported as part of our conversion.

Do you mean a different kind of automation?

What conversion in the server runtime?

#371 Updated by Greg Shah 8 months ago

What do you mean by this? SESSION:CURRENT-LANGUAGE is assignable and we react to it.

Greg, please, don't make me feel like an alien :)

</alien-mode>

Maybe I have to start from zero and explain why this whole thing has nothing to do with the sessionless translations I'm doing.

OK, I get that before login (after which the session is created), we don't have a way to assign it. For the purposes of login, we can live with the CURRENT-LANGUAGE as the default from the directory or we can take Sergey's advice and assign it based on the locale/language reported by the browser.

My point is that we are adding non-4GL translation support here and for the majority of the usage, we will have a SESSION and be able to honor any assignment of it.

In other words, full integration with our existing Translation Manager is very much preferred.

#372 Updated by Greg Shah 8 months ago

Galya B wrote:

Greg Shah wrote:

Having an external tool not plugged into the automation doesn't sound very standard for modern programs.

We do support this gettext usage from our converted code. In other words, the 4GL Translation Manager approach is fully supported as part of our conversion.

Do you mean a different kind of automation?

What conversion in the server runtime?

From my perspective, the critical automation related to gettext is handled at code conversion time, not at runtime. We gather all the translatable strings and create artifacts that can then be translated using industry standard tools, of which gettext is the most prominent and has a very wide selection of 3rd party tooling that will work with our output.

#373 Updated by Galya B 8 months ago

OK, alien mode activated:

I think... and write... and hopefully it will be read... and understood... a very important summary coming:

Hynek's code is the same as mine, but mine is way more polished and in the right place, because it's already used where it should be and it doesn't mix with the legacy non-related stuff.

Now! The only major difference is the resource file. Greg, the more you write, the more you convince me I did right.

Why?
Because of two simple reasons:
1. I don't find it a good idea to run customer apps re-conversion every time someone changes the wording of an error message in the web handler. I don't even think s/he will consider it necessary for obvious reasons.
2. This current "bundle" is served to the front-end where the text will not be used. We do performance optimizations, but better to avoid the need for.

#374 Updated by Galya B 8 months ago

A little background on things...

There are two ways to load resource bundles: from .java class extending java.util.ResourceBundle and .properties files.

gettext generates resource files in the .java format. It's not ground-breaking, just an additional step. Obviously it has integration with js and other side features, which is nice, if used. At the end all texts are loaded in a Map in ListResourceBundle, but they should be converted from the java contents (check ListResourceBundle.loadLookup).

The other bundle type PropertyResourceBundle simply loads the properties file in the same data structure - a lookup Map by reading the file on instantiation (which in my implementation will be done once for the app life).

So behind the scenes things are pretty much the same.

Do we want separate files for the new text? I believe yes. Hynek was willing to consider it. I obviously pointed out a few times why that is needed.

Hynek proposed a new dir com.goldencode.p2j.util.tr, so we're not really arguing about the need of a new place for the file.

So at the end of the day we're arguing about the format of the file and the dire need of adding one more step to the generation of it by forcing all apps to be reconverted every time Sergey adds a new port error message.

#375 Updated by Greg Shah 8 months ago

I don't find it a good idea to run customer apps re-conversion every time someone changes the wording of an error message in the web handler.

I agree and that is exactly how we have already implemented it. No one is suggesting that conversion would have to be run in that case. Customer application translatable strings are in application-specific bundles. FWD translatable strings would be in FWD-specific bundles and then two would ever meet or overlap.

#376 Updated by Galya B 8 months ago

Greg Shah wrote:

FWD translatable strings would be in FWD-specific bundles and then two would ever meet or overlap.

How is then that new bundle generated? By what script? And why, when it can just not be generated, if it's already present (as properties file). What's the benefit of additional step? What is the requirement for it?

#377 Updated by Sergey Ivanovskiy 8 months ago

Galya B wrote:

Greg Shah wrote:

FWD translatable strings would be in FWD-specific bundles and then two would ever meet or overlap.

How is then that new bundle generated? By what script? And why, when it can just not be generated, if it's already present (as properties file). What's the benefit of additional step? What is the requirement for it?

Please consider TransactionManager as a module to perform translation tasks and gettext standard as a requirement for translation resources.

#378 Updated by Greg Shah 8 months ago

FWD translatable strings would be in FWD-specific bundles and then two would ever meet or overlap.

How is then that new bundle generated?

Why do we need to generate the FWD bundle? Any time we add some text that is translatable, we can just add it to the bundle as part of our edits.

I'm sure we could also implement a process for generating the bundles on the Java side, but that has nothing to do with the conversion of application resources.

As noted above, I'd like to avoid properties files and stick with the same resource bundle approach as we use for the rest of our translations.

What's the benefit of additional step? What is the requirement for it?

There is none and we plan no such thing. I have the feeling that I am not understand something crucial that you are trying to convey. I'm sorry, if that is the case.

#379 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

OK, alien mode activated:

I think... and write... and hopefully it will be read... and understood... a very important summary coming:

Hynek's code is the same as mine, but mine is way more polished and in the right place, because it's already used where it should be and it doesn't mix with the legacy non-related stuff.

Extracting the runtime translation logic in a separate class is fine. Just bare in mind that the other instance will have to keep in sync and react on assignments to CURRENT-LANGUAGE.

1. I don't find it a good idea to run customer apps re-conversion every time someone changes the wording of an error message in the web handler. I don't even think s/he will consider it necessary for obvious reasons.

Reconversion isn't required on wording changes in FWD runtime.

#380 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

A little background on things...

There are two ways to load resource bundles: from .java class extending java.util.ResourceBundle and .properties files.

gettext generates resource files in the .java format. It's not ground-breaking, just an additional step. Obviously it has integration with js and other side features, which is nice, if used. At the end all texts are loaded in a Map in ListResourceBundle, but they should be converted from the java contents (check ListResourceBundle.loadLookup).

The other bundle type PropertyResourceBundle simply loads the properties file in the same data structure - a lookup Map by reading the file on instantiation (which in my implementation will be done once for the app life).

So behind the scenes things are pretty much the same.

Do we want separate files for the new text? I believe yes. Hynek was willing to consider it. I obviously pointed out a few times why that is needed.

Why the separate files are needed? The resource bundle will be part of the fwd jar so it will be readily available in both server and client. Where it gets tricky is JS, but that is already handled in Sergey's implementation.

#381 Updated by Hynek Cihlar 8 months ago

Greg Shah wrote:

FWD translatable strings would be in FWD-specific bundles and then two would ever meet or overlap.

How is then that new bundle generated?

Why do we need to generate the FWD bundle? Any time we add some text that is translatable, we can just add it to the bundle as part of our edits.

The bundles shouldn't be edited directly. They are generated with msgfmt, one of the tool provided by GNU gettext. The code it produces is highly optimized, it even chooses different implementations based on the number of entries in the bundle.

The official process is as follows:
1. The developer adds new translatable string: tr("This is a translatable string").
2. If the translation is not added, the result of the translation will be the same supplied string. I.e., the fallback is always English.
3. The sources are scanned and the translation file (*.po) is generated. This goes to the translator.
4. The translator translates the file.
5. The translated file is added back to the FWD sources and resource bundle is generated with msgfmt.

The obvious advantage is that you can externalize the translation process. Either pass it to a professional or do it in batches (like when new language is added and all the strings must be translated). Also you can quickly catch any translation errors and missing translations.

For single entries I suppose the process will be simplified:

1. New translatable string added in code.
2. PO files edited.
3. The bundles generated as part of FWD build or manually triggered by the developer.

The translation process will require some effort and process. We will either implement our own or stick to what is already there.

#382 Updated by Galya B 8 months ago

Greg Shah wrote:

There is none and we plan no such thing. I have the feeling that I am not understand something crucial that you are trying to convey. I'm sorry, if that is the case.

I don't understand where is the line between fiction and reality in any of the comments on the topic.

Looking at the code, all your comments sound like fiction, but then reading the comments themselves, they sound like explaining what's already present and how it should be simply extended. You asked me originally to integrate the login page with something... what it is is still unclear... gettext... a JavaScript library?! That's the only gettext in the project. And you continue force me to do something... some particular superior implementation based on a tool not present in the build scripts or the Java code.

#383 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

Greg Shah wrote:

There is none and we plan no such thing. I have the feeling that I am not understand something crucial that you are trying to convey. I'm sorry, if that is the case.

I don't understand where is the line between fiction and reality in any of the comments on the topic.

Looking at the code, all your comments sound like fiction, but then reading the comments themselves, they sound like explaining what's already present and how it should be simply extended. You asked me originally to integrate the login page with something... what it is is still unclear... gettext... a JavaScript library?! That's the only gettext in the project. And you continue force me to do something... some particular superior implementation based on a tool not present in the build scripts or the Java code.

As already mentioned we use gettext for translation support of the legacy code. But the requirements there are slightly different from what is required from the FWD runtime translation support.

#384 Updated by Galya B 8 months ago

Greg Shah wrote:

Having an external tool not plugged into the automation doesn't sound very standard for modern programs.

We do support this gettext usage from our converted code. In other words, the 4GL Translation Manager approach is fully supported as part of our conversion.

Hynek Cihlar wrote:

The sources are scanned and the translation file (*.po) is generated.

Manually by running po2json.js. There is not a .sh file for it. *.po intermediate files don't even live in the project. No wiki / readme for that process. One mention in a bugfix.

The translated file is added back to the FWD sources and resource bundle is generated with msgfmt.

msgfmt lives on the workstation (not the project) and is called by gen-bundles.sh. Manually run.

Greg, is this the full automation support you were arguing exists? Do you see the additional steps?

Hynek Cihlar wrote:

The code it produces is highly optimized, it even chooses different implementations based on the number of entries in the bundle.

I explained clearly how it works. The code it produces is java class extending java.util.ResourceBundle. On instantiation of the java class it loads in memory all texts as constants (see your own .diff with the generated file). Then loads them for a second time in memory in ListResourceBundle when ResourceBundle.getBundle is called. There is nothing highly optimized in the whole story.

Sergey Ivanovskiy wrote:

Please consider TransactionManager as a module to perform translation tasks and gettext standard as a requirement for translation resources.

And SecurityManager is module to perform security tasks with its 9500+ lines of code. Then let's have Fwd module to perform fwd tasks. Single responsibility is a principle since 2003. Mixing non-related functionality in one class is bad coupling.


Maybe not everyone understood well the requirements... I mean those before the passionate defending of gettext.js integration.

To translate all spawn error messages on the server without session and no SESSION:CURRENT-LANGUAGE. It should be done on the server to translate the msg before its logged to have the same text in the logs and the ui and make end-users the golden standard for debugging web spawn issues.

A little summary of what I get as action points from the fiction I read:

  • Call TranslationManager.tr("hard-coded fulltext as key") in all places Java classes generate spawn errors.
  • Invent / Find a tool to scan all .java files and generate .po files.
  • Automate the process by adding a new .sh file to be manually run.
  • Currently there are no .po files in the project, but stick to the same directory and file name.
  • Support many current languages sessionless on the server.

#385 Updated by Hynek Cihlar 8 months ago

Hynek Cihlar wrote:

Galya B wrote:

Greg Shah wrote:

There is none and we plan no such thing. I have the feeling that I am not understand something crucial that you are trying to convey. I'm sorry, if that is the case.

I don't understand where is the line between fiction and reality in any of the comments on the topic.

Looking at the code, all your comments sound like fiction, but then reading the comments themselves, they sound like explaining what's already present and how it should be simply extended. You asked me originally to integrate the login page with something... what it is is still unclear... gettext... a JavaScript library?! That's the only gettext in the project. And you continue force me to do something... some particular superior implementation based on a tool not present in the build scripts or the Java code.

As already mentioned we use gettext for translation support of the legacy code. But the requirements there are slightly different from what is required from the FWD runtime translation support.

To clarify more. There is currently no gettext library dependency declared in FWD, the reason is that it wasn't yet required. gettext tools generate Java bundles that don't require any 3rd parties unless you use the more advanced features it offers, like plural forms or translation context.

#386 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

Greg Shah wrote:

Having an external tool not plugged into the automation doesn't sound very standard for modern programs.

We do support this gettext usage from our converted code. In other words, the 4GL Translation Manager approach is fully supported as part of our conversion.

Hynek Cihlar wrote:

The sources are scanned and the translation file (*.po) is generated.

Manually by running po2json.js. There is not a .sh file for it. *.po intermediate files don't even live in the project. No wiki / readme for that process. One mention in a bugfix.

See Internationalization. There are references to legacy code translation.

The translated file is added back to the FWD sources and resource bundle is generated with msgfmt.

msgfmt lives on the workstation (not the project) and is called by gen-bundles.sh. Manually run.

Greg, is this the full automation support you were arguing exists? Do you see the additional steps?

See Internationalization. Obviously some steps can't be automated. Like when a translator must step in and do the actual translation.

Maybe not everyone understood well the requirements... I mean those before the passionate defending of gettext.js integration.

To translate all spawn error messages on the server without session and no SESSION:CURRENT-LANGUAGE.

TranslationManager already falls back to the directory. See private String currentLanguage = Utils.getDirectoryNodeString(null, "currentLanguage", "?"); in EnvironmentOps. It should probably also fall back to the JVM default language when directory service is not available as Sergey suggested.

A little summary of what I get as action points from the fiction I read:

  • Call TranslationManager.tr("hard-coded fulltext as key") in all places Java classes generate spawn errors.
  • Invent / Find a tool to scan all .java files and generate .po files.

Many tools already exist. A good one is poeditor.com.

  • Automate the process by adding a new .sh file to be manually run.

We want a new Gradle build target that will generate Java resource bundles from po files. This will be run automatically or on demand.

  • Currently there are no .po files in the project, but stick to the same directory and file name.

We want single po file for the whole core FWD runtime.

#387 Updated by Sergey Ivanovskiy 8 months ago

Galya B wrote:

what it is is still unclear... gettext... a JavaScript library?! That's the only gettext in the project. And you continue force me to do something... some particular superior implementation based on a tool not present in the build scripts or the Java code.

gettext is a project https://www.gnu.org/software/gettext/ but not java script library. gettext provides many front ends for developers depending on their projects and software languages used to build their products.

#388 Updated by Greg Shah 8 months ago

To be clear: I'm not expecting this task to do anything with automation for gettext. It is enough to create the .po file manually with the small number of strings needed for this task. We are just putting some initial runtime support in place here, not translating everything in the project.

The javascript side of our code does directly use gettext.js as Sergey detailed in #3931-290. The Java runtime side hasn't implemented anything yet, but we would intend to use the same approach as we use for the converted code. Implementing two different approaches is confusing to me and will generally lead to more bugs.

#389 Updated by Galya B 8 months ago

Thank you everyone for the contribution. Finally we can agree on my original comments - there is no tool, code, script or automation in place that needs to be "integrated" with the translation of the java constants.

If someone summarized it earlier, it would save us time:

We use po2json.js (with node.js) to find string constants in js files and generate .po files and then msgfmt to transform them to java classes of type ResourceBundle (Speaking about gettext was slightly confusing). All that is done manually on our workstations, nothing is in the Java or gradle code. And we want .po files for the Java constants. Also the new code should be in TranslationManager no matter how disjointed it is.

Good that I was able to figure this out.

#390 Updated by Galya B 8 months ago

Also the login page that can be server-side rendered, needs to load gettext.js and all resources and translate on the client although the user won't be able to change the language.

#391 Updated by Greg Shah 8 months ago

Galya B wrote:

Also the login page that can be server-side rendered, needs to load gettext.js and all resources and translate on the client although the user won't be able to change the language.

Why not do this on the server in Java and only provide what is needed? Why not use the browser's language/locale to set the default for the translations?

#392 Updated by Galya B 8 months ago

Greg Shah wrote:

Why not do this on the server in Java and only provide what is needed? Why not use the browser's language/locale to set the default for the translations?

we would intend to use the same approach as we use for the converted code. Implementing two different approaches is confusing to me and will generally lead to more bugs.

Well...
1. The spawn errors are not present in the front-end files and can't be automatically extracted to .po files. So you'll have that difference. As you mentioned, the .po files have to be created manually.
2. On GET /gui we can't determine the browser locale server-side, so all translated strings for all languages need to go to the client. Then gettext.js will be used to load them, like it is with the current approach. If the language was not expected to be determined based on the browser locale, then the page could be translated server-side.
3. If the translation is done in the front-end, we can receive the locale on POST login attempt and map the locale to an existing translation configuration, so that the logs will be in the same language. Such map (en_US / en_UK -> English) currently doesn't exist in the code. Then that locale need to be also passed in to the client and then back to the standardserver as initial current-language... or just let them live their own lives?

But if we're translating errors on the server for the logs anyways (that's a new), then why include them in the translation configuration sent to the client? Maybe we need two separate "bundles". And we'll be loading gettext.js to translate the 5 labels / buttons on the login/out pages. That's fine.

Setting / getting current language in EnvironmentOps doesn't work for without session. Here's another difference.

So, we'll be blocking the review now to add this new feature?

#393 Updated by Galya B 8 months ago

Also let me just conclude something again, because I'm not understood when I say it only once.

Having .po files means adding additional manual step every time a related (casually avoided to be considered related) error is added or wording changed. The additional step will provide one more additional file (.java) with the same constants that should also be uploaded to the vcs. Also the cool plural features, etc. of gettext can't be used server-side (not that they're needed) without additional library that supports gettext for Java.

#394 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

1. The spawn errors are not present in the front-end files and can't be automatically extracted to .po files. So you'll have that difference. As you mentioned, the .po files have to be created manually.

Why not to extract them from the source files where they are present?

#395 Updated by Galya B 8 months ago

Hynek Cihlar wrote:

Galya B wrote:

1. The spawn errors are not present in the front-end files and can't be automatically extracted to .po files. So you'll have that difference. As you mentioned, the .po files have to be created manually.

Why not to extract them from the source files where they are present?

I hope you're kidding.

#396 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

Also let me just conclude something again, because I'm not understood when I say it only once.

Having .po files means adding additional manual step every time a related (casually avoided to be considered related) error is added or wording changed.

This is no different from property files.... actually there is. With gettext you have the option not to care about po files and just generate them from the sources when needed.

With property files this step is mandatory. With every change to translatable strings you need to update the property files. More, you have to manage the keys, which quickly gets out of hands when the strings scale up.

The additional step will provide one more additional file (.java) with the same constants that should also be uploaded to the vcs. Also the cool plural features, etc. of gettext can't be used server-side (not that they're needed) without additional library that supports gettext for Java.

Beside the cool plural feature you also get context. For the cases when there is same source text translated differently based on context.

#397 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

Hynek Cihlar wrote:

Galya B wrote:

1. The spawn errors are not present in the front-end files and can't be automatically extracted to .po files. So you'll have that difference. As you mentioned, the .po files have to be created manually.

Why not to extract them from the source files where they are present?

I hope you're kidding.

Well just define the translations in the front end files (js or whatever). Generate the json bundles and use them directly in the frontend. Don't do the Java bundles for JS.

#398 Updated by Galya B 8 months ago

Which command do you use to create .po files from app sources?

#399 Updated by Hynek Cihlar 8 months ago

Galya B wrote:

Which command do you use to create .po files from app sources?

I always used POEditor. But I think you can use also xgettext: https://www.gnu.org/software/gettext/manual/gettext.html#xgettext-Invocation.

#400 Updated by Galya B 8 months ago

  • Related to Feature #7814: Supporting translated texts for web login / logout screens added

#401 Updated by Galya B 8 months ago

Getting back to the review.

There was an important question about what we do with trustedspawner and the fact that all os users without passwords should be set as resource-instance/reference and all fwd users, associated with os users without passwords, need to be present as subjects.

The easy solution is to remove the plugin, but remote is still present and it was said the check is important for remote spawn. But let's not forget that remote is a type of embedded and it should require SSO enabled as well, although at the moment it doesn't explicitly.

If we make SSO required for remote embedded, is it solving the problem? The passed-in in-app user/pass will be checked against the SsoAuthenticator before any spawn. Then we can remove trustedspawner?

#402 Updated by Galya B 8 months ago

Galya B wrote:

But let's not forget that remote is a type of embedded and it should require SSO enabled as well, although at the moment it doesn't explicitly.

My bad, even at the moment in 3931a it requires SSO enabled to register the network interface.

#403 Updated by Greg Shah 8 months ago

As I noted above #3931-322, I'm OK with removing the remote form of embedded and the trustedspawner. However, I want to hear Constantin's opinion.

#404 Updated by Constantin Asofiei 7 months ago

Review for 3931a rev 14781.

Admin Console - OS User
  • editing an OS user:
    • does not load the Description
    • password is lost even if 'password unchanged' is selected
  • pwsetdate and pwsettime is not set for a new user
  • disabling/enabling an user loses the password and description
  • in Accounts/Users, please add a column with the OS Username (if set)
  • needs javadoc: OsUsersPresenter.checkAssociatedFwdUsers

Greg: we have also the OS user configured for launching batch programs or appservers. This is done via clientConfig/systemUser and clientConfig/systemPassword - do we want to refactor this to use the OS user account specified at the process account, and remove the clientConfig settings? We can do this in a later branch, to not delay 3931a.

GUI Web mode
  • on default OS user login page, the ENTER key must submit the form to login - now you need to click on the 'Sign in' button
Embedded Web mode (local, not remote)
  • still using the SSO sample, embedded mode shows the login page (which requires what? OS user or app user?), but after SSO, it does not work with Hotel GUI and the SSO sample, at all. Do you have any changes in Hotel GUI to allow embedded mode (both remote and in-JVM) to work?
SSO - I'm testing with Hotel GUI; I've done this:
  • added trustedspawner/subjects=bogus
  • set these under auth-mode
              <node-attribute name="ssoplugin" value="com.goldencode.p2j.security.SampleSsoAuthenticator"/>
              <node-attribute name="ssopluginoption" value="bogus"/>
    
  • set defaultOsUser=ca
    and
  • on initial access, the username/password login page is still shown, but is this just because this is how the SSO auth sample works? Note that whatever values you enter for user/pw, they are not used at all.
  • after pressing 'Exit', I'm returned to a webpage which shows a You've been logout - back to login page link, and after clicking 'back to login', it just uses SSO to automatically log me again. Even if I'm restarting the FWD server, I'm still automatically logged in.
Questions:
  • do we need a way to invalidate all sessions on FWD server restart?
  • at least, we need a way to actually log off a user's session from SSO. How is the customer's SSO plugin supposed to do this? The sample doesn't show this.
Translation
  • as a note, the translation for these messages is separated from the converted app translated strings (which in my opinion is OK).
  • SessionlessTranslationManager.translate - this works with .properties file, and the argument is a key, not an actual text. Please change the param name and javadoc accordingly (we are not translating a text, but getting the text for a specified key).
  • the constructor assumes the customer's translation .properties files exist in the com.goldencode.p2j.translation package - I don't really like this. Greg: should we use SourceNameMapper.getPackageRoot() + "/translation" package, for customer-specific translation files?
  • we need nl_NL translation of WebDriverHandler.properties
Misc
  • ClientParameters has no actual change, just a history entry - please revert this file to its trunk rev.
  • SecurityManager.removeContext needs implementation.

Remote Launching

The discussion is mainly that for normal SSO spawning, we require that the FWD account is configured as trustedspawner. For remote launching, it requires trustedspawner and also remotelaunchoption to be configured. At this point, we have full control of the OS users which are used for spawning the web clients. I think is OK to remove the trustedspawner for the web client case, and leave it only for the remote launch case. This way, a context needs to be assigned (for security checks) only for remote launching.

#405 Updated by Galya B 7 months ago

Constantin Asofiei wrote:

Review for 3931a rev 14781.

Up in r14782: Admin console fixes and improvements; ENTER support for login.

Embedded Web mode (local, not remote)
  • still using the SSO sample, embedded mode shows the login page (which requires what? OS user or app user?), but after SSO, it does not work with Hotel GUI and the SSO sample, at all. Do you have any changes in Hotel GUI to allow embedded mode (both remote and in-JVM) to work?
SSO - I'm testing with Hotel GUI; I've done this:
  • added trustedspawner/subjects=bogus
  • set these under auth-mode
    [...]
  • set defaultOsUser=ca
    and
  • on initial access, the username/password login page is still shown, but is this just because this is how the SSO auth sample works? Note that whatever values you enter for user/pw, they are not used at all.

All embedded will work only with SSO, so the login will require only in-app user / app. Hotel GUI requires custom SsoAuthenticator, that knows what in-app credentials are in the DB. A dummy one can always return true and "bogus" as in #3931-274, but to make it proper the new authenticator should connect to the DB in the same way the 4GL code does. I can try to implement it later.

  • after pressing 'Exit', I'm returned to a webpage which shows a You've been logout - back to login page link, and after clicking 'back to login', it just uses SSO to automatically log me again. Even if I'm restarting the FWD server, I'm still automatically logged in.

Yes, that's why there is the logout page. Otherwise it's not needed.

Questions:
  • do we need a way to invalidate all sessions on FWD server restart?

SSO providers and auth servers usually use cookies with set expiration. In normal web apps these cookies are sent on each request and validated with each user action, and since http is stateless that works. In FWD once the user is in (after initial cookie/credentials validation), it's up to the 4GL app to decide/re-check if the session has expired, the same way they would check the validity of the AUTH-BLOB (authentication token). If a particular customer wants to access their own SsoAuthenticator methods from 4GL, we'll discuss it further.

There is another aspect of that topic - the mechanism of notifying the authenticator for an ended FWD session - and I've asked before in #4855-3, where Greg answered it's currently possible with certain hooks.

  • at least, we need a way to actually log off a user's session from SSO. How is the customer's SSO plugin supposed to do this? The sample doesn't show this.

How is it supported currently? I think logout is done in 4GL, which means the remote auth server will be notified by 4GL that this session is no longer valid and on the next login attempt, it will require credentials again instead of triggering auto-login.

#406 Updated by Galya B 7 months ago

Ah, I think what's confusing at the moment with the dummy SsoAuthenticator and hotel gui is that the authenticator allows all credentials, while the 4GL authenticaton does a follow-up check for the same credentials in the app DB. So both need to be synced to get proper error before launching the client. As I've said I need to spend some time to get familiar with the DB classes and schema to develop proper authenticator for hotel gui.

#407 Updated by Galya B 7 months ago

More on the topic of sessions:
  • FWD sessions: number of live sessions and their type (forked, same browser, etc), will be closely related to the licensing policies of the customers.
  • Web sessions (only applicable with auto-login implemented) can have expiration in weeks / months / the so called "session" (ended with browser exit) or even indefinite (obviously not recommended). It all depends on the cookie the customer devs can set themselves.

#408 Updated by Galya B 7 months ago

Constantin Asofiei wrote:

Remote Launching

The discussion is mainly that for normal SSO spawning, we require that the FWD account is configured as trustedspawner. For remote launching, it requires trustedspawner and also remotelaunchoption to be configured. At this point, we have full control of the OS users which are used for spawning the web clients. I think is OK to remove the trustedspawner for the web client case, and leave it only for the remote launch case. This way, a context needs to be assigned (for security checks) only for remote launching.

I still think trustedspawner is not needed even for remote spawn. Here is why:
  • embedded (including remote) will work only with SSO;
  • SSO validates the provided in-app user / pass against the SsoAuthenticator customer's implementation, so the credentials are validated instead of blindly spawning a client like it was before;
  • remotelaunchoption will check if the FWD user returned by the SsoAuthenticator have the rights to be used by remote calls;
  • the OS user associated with the FWD user is only used after the above two checks, and its origin (a mapping of SsoAuthenticator result to server configurations) is not from outside.

Basically at this point there is no doubt the os user is associated with a valid launch request.

#409 Updated by Constantin Asofiei 7 months ago

Galya, I agree about the trustedspawner - you are correct, the FWD user can have its own OS user and we can consider the security to be there.

About the SSO automatic-login and in general:
  • on initial login, I assume Map<String, String[]> paramMap contains the login parameters, too? And the plugin needs to know which key is for username/password? If so, can you add in Javadoc the standard names for the username/password fields?
  • on logout: this can be considered anytime when the 4GL application closes (can be via ALT-F4/browser close and normal 'logout/window close' button press). This just closes the FWD client, and on the Web browser side, it shows that logout page. But, there is no way for the user to explicitly 'I want to logoff with this application user from this browser, completely'. Here I don't mean 'cookie expiration date' or something like that, a full logout of that session from this browser. At this point, from your description it requires to have some equivalent application logic which gets executed on client termination, to invalidate the session at the application level, and the next SSO attempt will see this and show the login page. But, is there a way to do this without having to rely on another hook? It looks too complicated at this point and I'm thinking maybe we can find something easier.
Another description of this scenario is this:
  • the user works on their own computer, where the session can live for whatever time (i.e. login is performed on monday and the session expiries on friday, so is not needed to login every day).
  • the user goes to another computer, does some work, and after that wants to fully log off.

#410 Updated by Galya B 7 months ago

I would like to know the opinion of the customers who would implement auto-login, because it's very specific.

But you're right that "logout" from FWD session is not logout from web session and that's how it should be, but the phrasing on the logout page could be confusing then. I was originally thinking about the no-auto-login case, where it works well. What about rephrasing it to "The session has ended." and leave the button as it is "Back to Login Page" or "Start a new session".

#411 Updated by Galya B 7 months ago

Also, on the logout page there could be a second button called "Sign out", only when auto-login is enabled in the SsoAuthenticator. On click there could be a POST call to /logout that will call a new method in the authenticator that contacts the auth server to invalidate the session and then invalidates the cookie (by returning one with the same name, but expired) and FWD returns the login page with the newly expired cookie.

I think this is in line what you're asking for?

#412 Updated by Galya B 7 months ago

Constantin Asofiei wrote:

  • SecurityManager.removeContext needs implementation.

I tried everything and nothing else works except for SecurityContextStack.setContext(null);. Do we really need more cleanup? It was a minimalist session creation to start with; I guess that's why all the other cleanup methods I left commented out are throwing exceptions.

#413 Updated by Constantin Asofiei 7 months ago

Galya B wrote:

Constantin Asofiei wrote:

  • SecurityManager.removeContext needs implementation.

I tried everything and nothing else works except for SecurityContextStack.setContext(null);. Do we really need more cleanup? It was a minimalist session creation to start with; I guess that's why all the other cleanup methods I left commented out are throwing exceptions.

I think this is related to #7494-53, the SecurityManager.listOfSess leak. We need to ensure that everything is cleaned up. I'll look into it tomorrow.

#414 Updated by Greg Shah 7 months ago

Greg: we have also the OS user configured for launching batch programs or appservers. This is done via clientConfig/systemUser and clientConfig/systemPassword - do we want to refactor this to use the OS user account specified at the process account, and remove the clientConfig settings?

Yes, this makes sense.

We can do this in a later branch, to not delay 3931a.

Good plan. There are some other changes that will be following as well, so that will work.

do we need a way to invalidate all sessions on FWD server restart?

This makes sense.

Translation
  • as a note, the translation for these messages is separated from the converted app translated strings (which in my opinion is OK).

Yes

  • SessionlessTranslationManager.translate - this works with .properties file, and the argument is a key, not an actual text. Please change the param name and javadoc accordingly (we are not translating a text, but getting the text for a specified key).

We can keep the .po files separate from the main app but we do want to use the same approach to translation.

the constructor assumes the customer's translation .properties files exist in the com.goldencode.p2j.translation package - I don't really like this.
Greg: should we use SourceNameMapper.getPackageRoot() + "/translation" package, for customer-specific translation files?

Yes, it needs to come from the customer app.

Remote Launching

The discussion is mainly that for normal SSO spawning, we require that the FWD account is configured as trustedspawner. For remote launching, it requires trustedspawner and also remotelaunchoption to be configured. At this point, we have full control of the OS users which are used for spawning the web clients. I think is OK to remove the trustedspawner for the web client case, and leave it only for the remote launch case. This way, a context needs to be assigned (for security checks) only for remote launching.

Why not remove remote launching?

#415 Updated by Constantin Asofiei 7 months ago

Greg Shah wrote:

Why not remove remote launching?

This will make it mandatory to run any embedded web app from within the FWD Server's JVM. If we want to remove this option, then I'm OK with it.

#416 Updated by Constantin Asofiei 7 months ago

Constantin Asofiei wrote:

Greg Shah wrote:

Why not remove remote launching?

This will make it mandatory to run any embedded web app from within the FWD Server's JVM. If we want to remove this option, then I'm OK with it.

Actually, I've misunderstood something; 'embedded remote' relies on SSO, too, so the received user/pw combo gets passed to the SSO plugin, which validates (as app credentials) and returns a FWD user - and this has an OS user attached. So we are in the same security checks for normal Web client with SSO.

Galya: I think we need to change the javadoc for EmbeddedWebHandler.spawn and WebClientLauncher to reflect that the username/password are no longer OS credentials, but always application credentials. Embedded can not work without SSO.

#417 Updated by Galya B 7 months ago

Constantin Asofiei wrote:

Galya: I think we need to change the javadoc for EmbeddedWebHandler.spawn and WebClientLauncher to reflect that the username/password are no longer OS credentials, but always application credentials. Embedded can not work without SSO.

That's right. I'm fixing it.

Greg Shah wrote:

do we need a way to invalidate all sessions on FWD server restart?

This makes sense.

What do you mean by that? Notify the authenticator of the server being started to allow it cleanup records of live FWD sessions? The customer can do it themselves in the default authenticator constructor. In theory it's called once for the lifetime of the server, but they can also add an AtomicBoolean to keep track.

  • SessionlessTranslationManager.translate - this works with .properties file, and the argument is a key, not an actual text. Please change the param name and javadoc accordingly (we are not translating a text, but getting the text for a specified key).

We can keep the .po files separate from the main app but we do want to use the same approach to translation.

I wrote a few thesis on the topic in #7814 and I think we need another one. There is some major misunderstanding in how things are currently working and how that applies to the current case. Wait for a more visual explanation soon.

Why not remove remote launching?

As Constantin stated, currently the remote interface has the same level of security as web (if not tighter with remotelaunchoption), because of the requirement for SSO. I'm fixing the javadoc to state that.

#418 Updated by Constantin Asofiei 7 months ago

Galya B wrote:

What do you mean by that? Notify the authenticator of the server being started to allow it cleanup records of live FWD sessions? The customer can do it themselves in the default authenticator constructor. In theory it's called once for the lifetime of the server, but they can also add an AtomicBoolean to keep track.

Considering that FWD has no control over what auth cookies the SSO plugin adds, I think we just need to specify some scenarios to determine that the FWD server has restarted. One way would be as you say - in the constructor logout all active sessions in the application, so the SSO plugin will not auto-login. Another way would be to use a cookie which sets a UUID (computed on server start, in the SSO plugin constructor) and check this cookie if this is the same FWD server instance. I should be enough to document these approaches in the SSO interface javadoc.

#419 Updated by Galya B 7 months ago

Constantin Asofiei wrote:

Galya B wrote:

What do you mean by that? Notify the authenticator of the server being started to allow it cleanup records of live FWD sessions? The customer can do it themselves in the default authenticator constructor. In theory it's called once for the lifetime of the server, but they can also add an AtomicBoolean to keep track.

Considering that FWD has no control over what auth cookies the SSO plugin adds, I think we just need to specify some scenarios to determine that the FWD server has restarted. One way would be as you say - in the constructor logout all active sessions in the application, so the SSO plugin will not auto-login. Another way would be to use a cookie which sets a UUID (computed on server start, in the SSO plugin constructor) and check this cookie if this is the same FWD server instance. I should be enough to document these approaches in the SSO interface javadoc.

I'm pretty sure the web session should persist between FWD restarts. It's not related to the FWD lifecycle. The only relationship is the licensing policies. The auto-login is for end-user convenience and can't be driven by the server.

Basically the web session with auto-login is only for authentication - identifying the end-user, while the FWD session and browser / machine details are related to licensing.

I'll add the details on how to track FWD sessions in the doc.

#420 Updated by Galya B 7 months ago

Auto-login session expiration is driven by the 3rd party authentication server / directory. Adding Sign Out button on the logout page it can also be influenced by the end-user on intentional action.

#421 Updated by Galya B 7 months ago

I've added the section Enforcing Licensing Policies. I think it contains all the needed information.

#422 Updated by Greg Shah 7 months ago

Why not remove remote launching?

This will make it mandatory to run any embedded web app from within the FWD Server's JVM. If we want to remove this option, then I'm OK with it.

Yes, I understand. I just see problems with remote mode.

  • If the embedded web client itself loads from another web server, it means that it can easily be mismatched with our implementation. That mismatch could happen based on not updating the code to match the FWD level OR it could happen that someone deliberately edits the code. This could lead to:
    • Subtle bugs that are hard to diagnose and fix.
    • Security problems introduced by an implementation that previously would have been safe to rely upon.
  • More complex support code and testing in FWD, which increases our long term costs and effort.

On the "positive side" the remote mode would be more natural for the case where there is an existing external web application in which:

  • The legacy screens needed to be accessible but did not constitute the majority of the application; AND
  • The external web application had other reasons that it needed a direct RemoteObject connection to the FWD server (e.g. to call appserver APIs).

I just wonder if the negatives outweigh the positive in this one case.

#423 Updated by Galya B 7 months ago

I'm against having the remote interface, because it will break any of the enhancements to the wrapper container I'm going to introduce like persistent client storage, registry, browser fingerprint, etc.

It's 10 sec work to remove it, adding it back will take 10 min. I'll remove it right after the last rebase on the branch, before merge. That way it can easily be found and restored if needed later.

#424 Updated by Constantin Asofiei 7 months ago

Galya, go ahead and remove the RemoteObject.registerStaticNetworkServer code for WebClientLauncher in StandardServer, the remote-related code in WebDriverHandler.spawnWorker - so neither the trusted and remote code is needed, plus the cleanupJettyThread method and related code.

#425 Updated by Galya B 7 months ago

Done in r14784.

#426 Updated by Galya B 7 months ago

We're not removing RemoteLaunchOptionRightsEditor and related code now, although it won't have any effect. I heard Greg wants to implement some type of client-side defined options in the future, that can reuse the security plugin, so leaving the code for now.

#427 Updated by Galya B 7 months ago

I pushed small, but important fix for embedded handlers in r14786. To serve the hotel_gui login page as before, on top of the notes in #3931-274 you should also:

  • move index.html from embedded/src/resources/ to webres/ and rename it to web_partial_login.html.
  • add to build.xml:
       <target name="prepare" depends="prepare.embedded">
    
         [..]
    
          <!-- copy base web res dir -->
          <copy todir="${build.home}/classes/${pkgroot}">
             <fileset dir="${fwd.home}">
                <include name="webres/**"/>
             </fileset>
          </copy>
    
         [..]
    
       </target>
    

#428 Updated by Galya B 7 months ago

Do I rebase and we do the final review?

Login / logout pages are not translated. That can go under #4762 or #7814.

Also my work on hotel_gui SsoAuthenticator is not related to the merge of 3931a.

#429 Updated by Constantin Asofiei 7 months ago

I'll do the review first and will keep the rebase when we are close to merge (as I assume it can be kind of difficult).

In the mean time, please finish the documentation and the Hotel GUI changes to work with 3931a. These need to be released with 3931a.

#430 Updated by Galya B 7 months ago

For the hotel_gui authenticator there should be a connection to the DB and then SecurityOps.setUserId(username, password) can be used to verify the credentials. But ConnectionManager.getDefaultDatabase uses ContextLocal connected and there is no context on login attempt. I tried to add context, but it also requested session:

com.goldencode.p2j.persist.PersistenceException: Error populating metadata database local/hotel/meta
Caused by: java.lang.NullPointerException
    at com.goldencode.p2j.ui.LogicalTerminal.init(LogicalTerminal.java:1557)
    at com.goldencode.p2j.security.ContextLocal.initializeValue(ContextLocal.java:704)

Any hint how to achieve the same functionality without context and session?

#431 Updated by Galya B 7 months ago

Where is the meta_user table, where is the h2 db - the file hotel_gui/deploy/db/hotel.h2.db seems empty. Is there another embedded h2 db?

#432 Updated by Galya B 7 months ago

I created a simple jdbc connection in the authenticator, but SELECT COUNT(*) FROM meta_user returns 0. When is meta_user populated with data/dump/hotel/_user.d?

#433 Updated by Galya B 7 months ago

The user password has to be encoded, the result is found.

#434 Updated by Galya B 7 months ago

I see ConnectionManager tries to be smart and find the column names for user / pass based on com.goldencode.p2j.persist.dialect.Dialect. Do we need that in hotel_gui SsoAuthenticator implementation or hardcoding SELECT COUNT(*) as count FROM meta_user WHERE userid = upper(rtrim('')) AND password = ''; is good enough?

#435 Updated by Galya B 7 months ago

I still don't know the answer of the last question, but what I can do on hotel_gui is done. The SsoAuthenticator implementation is attached. Since there is no actual auth server with web session token and cookies, what we have is DB verification of credentials both before launch (with POST /embedded, calling HotelGuiSsoAuthenticator, querying meta_user) and after client launch the code stays the same - the front-end invokes 4GL procedure doLogin with the same preserved credentials to verify on the client end the login and set the app user.

      <node class="container" name="config">
        <node class="authMode" name="auth-mode">
          <node-attribute name="mode" value="4"/>
          <node-attribute name="retries" value="-1"/>
          <node-attribute name="plugin" value="com.goldencode.p2j.security.GuestAccess"/>
          <node-attribute name="ssoplugin" value="com.goldencode.hotel.HotelGuiSsoAuthenticator"/>
          <node-attribute name="ssopluginoption" value="bogus"/>
        </node>
      </node>

r14787 some related tweaks.

I'm done. Something missing?

#436 Updated by Galya B 7 months ago

P.S. HotelGuiSsoAuthenticator.java seems to have to be placed in embedded/src/com/goldencode/hotel, but it will get eventually copied by some build script to src/com/goldencode/hotel and that's where it should be to be found on compilation.

#437 Updated by Greg Shah 7 months ago

Ovidiu: Can you please review starting at #3931-430 and advise? With this task we are moving Hotel GUI away from 4GL login code to a hand-coded Java implementation of an authenticator. This will serve as a useful example for our customers use of SSO.

#438 Updated by Ovidiu Maxiniuc 7 months ago

I do not have a solution currently. Just some notes, until I read the whole thread and get a better understanding of the issue.

I see the proposed SSO Authenticator is very similar to ConnectionManager.authenticate(). I assume its invocation is the reason of the NPE from #3931-430. However, I think we might need to expose some API for Authenticators which should hide the tables and fields.

At the moment, the authentication requires a specific database (against which the provide credentials are validated), and the intermediary code use the first of the database loaded at startup. Certainly, this will change. Also, the databases (if the application use two or more) may contain different sets of users or users with different passwords. This need to be clarified. We tried to implement at a certain moment the concept of "default database" for similar reasons, but I am still not sure there is such thing in OE.

#439 Updated by Galya B 7 months ago

Ovidiu, a little bit of context:

SsoAuthenticator is an interface for implementation by the customer with customer specific code related to how in-app user credentials are verified. This can vary widely: in-app credentials verification can be done by accessing any 3rd party directory or athentication server. SsoAuthenticator methods are called by the web handlers for POST requests to /gui, /chui, /embedded, basically on attempt for login + launching a new web client process. All this is running on the embedded Jetty server threads on the server and there is no session with the client, so there is no security context and ContextLocal can't be accessed.

SsoAuthenticator for hotel_gui should be an example of how the customer will write their code to verify the in-app credentials against a DB. Previously it worked so: the login form was sending an unauthorized request GET /embedded/launch and a new client process was spawned. The login form user / pass were then sent from the front-end to the client process -> to the server process with invocation of 4GL method doLogin. doLogin used SecurityOps.setUserId(username, password) to validate the credentials.

I wanted to reproduce the same logic in HotelGuiSsoAuthenticator, before the client process is spawned, that is on the sessionless Jetty thread, but I couldn't use directly ConnectionManager, because it relies on ContextLocal.

The solution I attached in HotelGuiSsoAuthenticator is based on simple JDBC connection / query using the directory configs and is most likely the way customers will implement it themselves. It works with h2 DB, that's tested.

But when duplicating the logic from ConnectionManager.authenticate(), I've noticed that the query uses dynamic column names instead of directly hard-coded userid, password. Is this a valid case for hotel_gui that should be taken into consideration?

#440 Updated by Greg Shah 7 months ago

For this use case where the original code was using 4GL features for database authentication, I think we should implement Ovidiu's idea of a helper (or helpers) to hide the details of our low level persistence design. As Galya notes, this will require us to implement some code that is sessionless (does not depend on an existing session/context-local state). If we don't do this, then the customer will have to change their code every time we implement some low level changes and they will have to write different versions of this code for different database back ends. It doesn't make sense when we already maintain such a layer in our persistence support.

#441 Updated by Galya B 7 months ago

Can we compile a list of authentication methods in OpenEdge 4GL that can be applicable?

I know only of setuserid. Its implementation relies on DatabaseManager, ConnectionManager, Dialect and other classes using session and context.

Do we block 3931a with the rework of the relevant parts to accommodate for hotel_gui proper authenticator?

#442 Updated by Greg Shah 7 months ago

I'm not opposed to doing this in the next set of changes (which have to be completed quickly anyway due to customer deadlines) but this will work only if we don't break any existing projects when we merge.

We can't discuss customer specifics in this task. Are all necessary migration steps documented?

#443 Updated by Galya B 7 months ago

Greg Shah wrote:

I'm not opposed to doing this in the next set of changes (which have to be completed quickly anyway due to customer deadlines) but this will work only if we don't break any existing projects when we merge.

This can be important for migrating embedded projects, I guess, especially having the future in mind, so that the proposed implementation of hotel_gui sso doesn't get broken, but even for embedded it should work for now. So I don't think it should be that quickly.

We can't discuss customer specifics in this task. Are all necessary migration steps documented?

Everything I could think of is documented in the wiki. As I've said before, that's for virtual desktop to continue working without SSO. SSO should be implemented by the customer. I'm not well familiar with all the cases, but I know for sure that at least one of them is not using setuserid or another 4GL method as the authentication method on the login page.

#444 Updated by Galya B 7 months ago

I imagine after merging 3931a, we'll focus on closing #4571 and #7067, and after that formulate the new requirements for 3931b.

#445 Updated by Ovidiu Maxiniuc 7 months ago

Galya B wrote:

But when duplicating the logic from ConnectionManager.authenticate(), I've noticed that the query uses dynamic column names instead of directly hard-coded userid, password. Is this a valid case for hotel_gui that should be taken into consideration?

In 4GL, the user-name is case-insensitive. But the password is not. Also the compare operations are performed on trimmed character values. The code you see in authenticate() is really generic. It does not assume one or another, but makes use of persistence boilerplate for natural access to these fields. Also this is dialect specific: PSQL allows functions calls, including in index definitions, H2 - OTOH - makes use of pre-computed columns to achieve the same goal. In the latter case, instead of applying upper(rtrim(fld)) (as for PSQL), the __ifld is used, as it is already processed.

#446 Updated by Galya B 7 months ago

The question is if there are customers using any 4GL methods like setuserid for login page authentication: in-app credentials validation? They are free to use it as before on any next step in 4GL, when the client process is spawned. If this is too customer-specific, please answer via email.

If there are none, I would ask for final review and rebase.

#447 Updated by Galya B 7 months ago

  • Related to Feature #7884: Standard 4GL methods used for login screen authentication to be exposed in sessionless helper classes added

#448 Updated by Galya B 7 months ago

I've committed r14788 - r14791 with improvements to fwd_sdk.js: documentation on functions and an exposed sendAuthRequest function to allow customers without login forms to still reuse FWD's request generation, that will allow some enhancements in the future.

#449 Updated by Constantin Asofiei 7 months ago

Galya, I think 3931a rev 14791 is OK. What needs to be done before merging this to trunk:
  • rebase 3931a
  • finish the documentation: all new directory/bootstrap config options need to be documented (emphasize what has been renamed and what we need to migrate existing customers to these new values, to keep the non-SSO login approach)
  • work on the changes for #5731 project (create a task in project:"Regression Testing" project to discuss this)
  • finish the sample SSO for Hotel GUI, and make sure it works for both embedded and normal Web clients
  • test with some customer apps to determine if any changes are needed to keep the non-SSO way for the Web client

#450 Updated by Galya B 7 months ago

Constantin Asofiei wrote:

  • finish the documentation: all new directory/bootstrap config options need to be documented (emphasize what has been renamed and what we need to migrate existing customers to these new values, to keep the non-SSO login approach)

This is part of Migration, but I've just added description to all related configs in Configurations.

#451 Updated by Galya B 7 months ago

Constantin Asofiei wrote:

  • finish the sample SSO for Hotel GUI, and make sure it works for both embedded and normal Web clients

The implementation in #3931-435 works at the moment with H2, but long term we need #7884 that still doesn't have a compiled requirements and estimate.
But there is something else. Virtual desktop and embedded use the same resource for the login page. It is overridden in hotel_gui to have the previous custom page for embedded, but this will break virtual desktop. The question is if it's OK to have only one of the modes running at the same time?

#452 Updated by Galya B 7 months ago

3931a rebased on trunk r14770.

#453 Updated by Greg Shah 7 months ago

Virtual desktop and embedded use the same resource for the login page. It is overridden in hotel_gui to have the previous custom page for embedded, but this will break virtual desktop. The question is if it's OK to have only one of the modes running at the same time?

No, that isn't OK. We need both modes to work simultaneously. Customers will need this in their applications as well.

#454 Updated by Galya B 7 months ago

Greg Shah wrote:

Virtual desktop and embedded use the same resource for the login page. It is overridden in hotel_gui to have the previous custom page for embedded, but this will break virtual desktop. The question is if it's OK to have only one of the modes running at the same time?

No, that isn't OK. We need both modes to work simultaneously. Customers will need this in their applications as well.

OK, fixed in r14855. Default embedded and VD login pages are separate resources.

#455 Updated by Galya B 7 months ago

The migration plan should assess the production / live system and answer several questions:

  1. Is /embedded mode in use:
    1. Positive: SSO needs to be implemented.
    2. Negative: no action needed. Can remove redundant directory config embeddedWebApp.
  2. Is the background on the login page or the virtual desktop branded by the customer logo:
    1. Positive: copy the customer logo in a new root/webres/ dir and rename it to logo.png. Add the copy folder build script (check the wiki).
    2. Negative: no action needed. Can remove redundant directory configs webClient/login-form and webClient/desktop.
  3. Does the customer's app Java source code contain references to SecurityManager:
    1. Positive: rework the references.
    2. Negative: no action needed.
  4. Are there any web UI tests?
    1. Positive: determine who and when is going to rework them to keep them running.
    2. Negative: no action needed.
  5. Required directory config changes:
    1. rename webClient/virtualDesktopEnabled to webClient/virtualDesktop.
  6. Recommended / optional directory config changes:
    1. remove webClient/enabled;
    2. remove trustedspawner configs;
    3. remove remotelaunchoption configs;
    4. remove embeddedWebApp;
    5. remove webClient/login-form and webClient/desktop;
  7. Re-conversion: Not needed currently. Will be needed when SSO is implemented and the 4GL code uses SESSION:AUTH-BLOB.

And don't forget to inform the customer about the new design of the OS login screen.

Greg, I need someone to answer these questions about the live / production systems (relevant systems that will be updated soon after the merge).

#456 Updated by Constantin Asofiei 7 months ago

Galya, full reconversion is required because progress.g adds a new token - this invalidates token IDs in schema AST files and leads to unexpected issues for dynamic queries/temp-tables.

Also, I just noticed something I missed: in progress.g, please comment AUTH-BLOB like this:

line 3714: KW_AUTHBLOB; // FWD extension, not real 4GL!

#457 Updated by Greg Shah 7 months ago

Can we compile a list of authentication methods in OpenEdge 4GL that can be applicable?

I know only of setuserid. Its implementation relies on DatabaseManager, ConnectionManager, Dialect and other classes using session and context.

  • "Legacy" style authentication
    • This is based on database level authentication.
    • There are two ways to do it:
      • SETUSERID()
      • CONNECT statement that includes -U (userid) and -P (password) connection parameters is a kind of implicit SETUSERID() for that database.
  • "Modern" approach
    • CLIENT-PRINCIPAL object in cooperation with some SECURITY-POLICY features. The CLIENT-PRINCIPAL is a session-level security token.
    • Two ways to do it:
      • Using an OpenEdge SSO facility. This has to be configured external to the session. Then the CLIENT-PRINCIPAL is setup using SECURITY-POLICY:SET-CLIENT() or the built-in function SET-DB-CLIENT().
      • DIY (do it yourself) using CLIENT-PRINCIPAL:SEAL().

FYI, we have partial support for CLIENT-PRINCIPAL but not for the SSO modes. See #3752, #3810, #4380 and #6423.

Do we block 3931a with the rework of the relevant parts to accommodate for hotel_gui proper authenticator?

We don't want to wait too long for it. I think your current approach has been good. The bottom line is that Hotel GUI needs to keep running properly in all its modes. Hotel ChUI needs to keep running too but Iguess your changes are less likely to break that.

#458 Updated by Galya B 7 months ago

Constantin Asofiei wrote:

Galya, full reconversion is required because progress.g adds a new token - this invalidates token IDs in schema AST files and leads to unexpected issues for dynamic queries/temp-tables.

I see. I was also wondering if re-conversion is needed for the changes in SecurityManager. If any SecurityManager references end up in the converted code, I guess so.

Also, I just noticed something I missed: in progress.g, please comment AUTH-BLOB like this:

line 3714: KW_AUTHBLOB; // FWD extension, not real 4GL!

Done in r14857.

#460 Updated by Galya B 7 months ago

  • Status changed from Review to Test

#461 Updated by Galya B 7 months ago

3931a was merged to trunk as rev. 14783 and archived.

#462 Updated by Eugenie Lyzenko 7 months ago

Galya,

Should the following entries also be removed from directory.xml?

...
        <node class="strings" name="resource-plugins">
          <node-attribute name="values" value="com.goldencode.p2j.main.TrustedSpawnerResource"/>
...
          <node-attribute name="values" value="com.goldencode.p2j.main.RemoteLaunchOptionResource"/>
        </node>
...

#463 Updated by Galya B 7 months ago

Eugenie Lyzenko wrote:

Galya,

Should the following entries also be removed from directory.xml?

[...]

Yes, these can be removed.

#464 Updated by Guido Brandsen 6 months ago

Galya, today we saw a minor problem with client spawning.
The error shown in OS login was:

The spawn process failed with %d error. Please see spawner logs.

There is a client base crash log (fwd_client_base_crash_20231031_085040.log), which shows this error:

23/10/31 08:50:40.527+0100 | WARNING | com.goldencode.p2j.util.Utils | ThreadName:main | !!! Disabled SSL certificate validation for outgoing HTTPS connections. This is intended for test environments only !!!
23/10/31 08:50:40.528+0100 |  SEVERE | com.goldencode.p2j.main.CommonDriver | ThreadName:main | System exits with code -13 caused by:
java.lang.UnsatisfiedLinkError: /v1/metacom/server/lib/libp2j.so: libffi.so.7: cannot open shared object file: No such file or directory
        at java.lang.ClassLoader$NativeLibrary.load(Native Method)
        at java.lang.ClassLoader.loadLibrary0(ClassLoader.java:1934)
        at java.lang.ClassLoader.loadLibrary(ClassLoader.java:1850)
        at java.lang.Runtime.loadLibrary0(Runtime.java:843)
        at java.lang.System.loadLibrary(System.java:1136)
        at com.goldencode.p2j.main.ClientCore.loadNativeLibrary(ClientCore.java:604)
        at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:471)
        at com.goldencode.p2j.main.ClientDriver.start(ClientDriver.java:272)
        at com.goldencode.p2j.main.CommonDriver.process(CommonDriver.java:579)
        at com.goldencode.p2j.main.ClientDriver.process(ClientDriver.java:349)
        at com.goldencode.p2j.main.ClientDriver.main(ClientDriver.java:394)

Can you fix that -13 is shown instead of %d?

Note: we already solved the real spawn problem. This comment is just about the string formatting.

#465 Updated by Galya B 6 months ago

Guido Brandsen wrote:

Galya, today we saw a minor problem with client spawning.
The error shown in OS login was:
[...]

There is a client base crash log (fwd_client_base_crash_20231031_085040.log), which shows this error:
[...]

Can you fix that -13 is shown instead of %d?

Note: we already solved the real spawn problem. This comment is just about the string formatting.

Yes, I fixed it in 4854a. It will probably get merged soon and I'll let you know.

#466 Updated by Galya B 6 months ago

Galya B wrote:

Guido Brandsen wrote:

Galya, today we saw a minor problem with client spawning.
The error shown in OS login was:
[...]

There is a client base crash log (fwd_client_base_crash_20231031_085040.log), which shows this error:
[...]

Can you fix that -13 is shown instead of %d?

Note: we already solved the real spawn problem. This comment is just about the string formatting.

Yes, I fixed it in 4854a. It will probably get merged soon and I'll let you know.

4854a was merged to trunk as rev. 14823.

#467 Updated by Galya B 6 months ago

I think we should reference the feature as simple sign-on (SSO). This way we'll be keeping all the SSO references, but will be dispersing the wrong impressions of what it is. Although it can be SSO in its usual meaning, I think this will be better for the marketing. Just a note.

#468 Updated by Galya B 6 months ago

  • Related to Bug #8074: Preferred UI Theme not selected in web with auto-login and forked sessions added

#469 Updated by Greg Shah 3 months ago

  • Status changed from Test to Closed

Also available in: Atom PDF