FWD » Core Development » Runtime Infrastructure

Moving to a REST API will provide several benefits.

• It will be a simpler and cleaner model than the current synchronous RPC approach.
• It will enable:
  • The creation of a command line interface that can handle anything the admin console can do.
  • Customers or 3rd parties will be able to write custom tools to solve problems which we have not otherwise resolved.

A small clarification. The Admin remoting isn't based on RemoteObject since the GWT rewrite.

There are three (GWT) servlets AdminServiceImpl, AuthServiceImpl and ReportServlet in the package com.goldencode.p2j.admin.server. The client-server channel is based on HTTP post requests with GWT specific serialization.

Have you thought about authentication for the REST API?

Have you thought about authentication for the REST API?

In #6229 we have implemented the first set REST APIs for admin purposes. This is in trunk now. Please see Tenant Administration API. It documents the current authentication approach. We can discuss changes here.

The wiki says '/admin/tenant/login. No header or body required.'. I don't see this in the code or testing it. BasicAuth requires Authorization header.

I would have expected it to be /admin/login.

It is indeed admin/tenant/login as the doc says, but I don't see provisioning for anonymous login without Authorization header.

Radu, would you confirm how the endpoint admin/tenant/login is supposed to be used. Please check the above notes.

Galya B wrote:

Radu, would you confirm how the endpoint admin/tenant/login is supposed to be used. Please check the above notes.

The things that are written in the Tenant Administration API wiki don't take into account what's needed to use the admin console.

In terms of login, in the environment I had for testing, I used Basic Auth with the username and password taken from directory.xml, under the <node class="container" name="users"> node (for testing I used bogus). After that, for the other requests, I had no Authorization, but I made sure to include in the header of the requests the FwdSessionId token that was generated at login. And, for these requests (other than the login one), I made sure that I had in the directory the nodes with ACL, found in Add Permissions for the Tenant Admin API. Also, the implementation for these tenant requests can be found in TenantConfig.

Also, I believe admin/login is used for logging in the admin console, while admin/tenant/login is for getting access to the multi-tenant stuff. You can't access admin/tenant/login without having performed the admin console login first.

Please let me know if you needed something else from me.

Radu Apetrii wrote:

Galya B wrote:

Radu, would you confirm how the endpoint admin/tenant/login is supposed to be used. Please check the above notes.

The things that are written in the Tenant Administration API wiki don't take into account what's needed to use the admin console.

In terms of login, in the environment I had for testing, I used Basic Auth with the username and password taken from directory.xml, under the <node class="container" name="users"> node (for testing I used bogus). After that, for the other requests, I had no Authorization, but I made sure to include in the header of the requests the FwdSessionId token that was generated at login. And, for these requests (other than the login one), I made sure that I had in the directory the nodes with ACL, found in Add Permissions for the Tenant Admin API. Also, the implementation for these tenant requests can be found in TenantConfig.

Also, I believe admin/login is used for logging in the admin console, while admin/tenant/login is for getting access to the multi-tenant stuff. You can't access admin/tenant/login without having performed the admin console login first.

Please let me know if you needed something else from me.

Would you please add this info to the wiki. Currently it says the tenant login request doesn't require any headers.

Yes, I will add this to the wiki. However, there's one thing I'm unsure about. These users that are in the <node class="container" name="users"> node in the directory, are these added by clients, or do we have some default ones, or can we ask the clients to add specific users? Greg/Galya: Can I get a bit of insight on these users? For reference, in the directory, they are found under accounts/users/<user_name> nodes.

Also, I believe admin/login is used for logging in the admin console, while admin/tenant/login is for getting access to the multi-tenant stuff. You can't access admin/tenant/login without having performed the admin console login first.

Hmm. The GWT handlers use this? I'd prefer to have them move to a different endpoint so that we can keep the REST API authentication paths logical.

In other words, I want the admin console to use something else and the REST API to use /admin/login. The token you get back from /admin/login should be enough to use any REST API, not just multi-tenant. There would be no /admin/tenant/login in my optimal world.

Radu Apetrii wrote:

Yes, I will add this to the wiki. However, there's one thing I'm unsure about. These users that are in the <node class="container" name="users"> node in the directory, are these added by clients, or do we have some default ones, or can we ask the clients to add specific users? Greg/Galya: Can I get a bit of insight on these users? For reference, in the directory, they are found under accounts/users/<user_name> nodes.

The users are configured by the customers. This can be done in the admin console under Accounts -> Users. Most customers have bogus as a default FWD account for the guest login, but it can be anything.

Radu Apetrii wrote:

Yes, I will add this to the wiki. However, there's one thing I'm unsure about. These users that are in the <node class="container" name="users"> node in the directory, are these added by clients, or do we have some default ones, or can we ask the clients to add specific users? Greg/Galya: Can I get a bit of insight on these users? For reference, in the directory, they are found under accounts/users/<user_name> nodes.

These users are all customer-created, though we certainly have a small number that show up in our standard Hotel examples and customers often migrate those into their own environments.

For the concepts, please see Security Design. One thing I don't like about our current design is that that have a difference between a process account and a user account. I think it should just be a user account with flags that tell us if something is a process or an interactive user. Regardless, hopefully that document helps provide some context.

Also, please see Accounts.

Greg Shah wrote:

Also, I believe admin/login is used for logging in the admin console, while admin/tenant/login is for getting access to the multi-tenant stuff. You can't access admin/tenant/login without having performed the admin console login first.

Hmm. The GWT handlers use this? I'd prefer to have them move to a different endpoint so that we can keep the REST API authentication paths logical.

GWT is currently authenticated with a request to /admin/adminapp/AuthService and it seems the JSESSIONID generated by tomcat for the anonymous session is linked to the authentication in the servlet context in the container, so there is no other explicit token going back and forth between the web client and the server.

The tenant api uses BasicAuth for authentication that accepts a FWD account's username and its webServiceToken as a password. These are plain text in directory. I don't think we should want to build a whole admin REST API with the credentials available to anyone with access to directory.xml.

Galya B wrote:

The tenant api uses BasicAuth for authentication that accepts a FWD account's username and its webServiceToken as a password. These are plain text in directory. I don't think we should want to build a whole admin REST API with the credentials available to anyone with access to directory.xml.

I think this was just a quick and dirty solution. We need a proper solution to be implemented, which would authenticate with a FWD account and provide a token back that can be used for subsequent API calls.

The authentication method should be configurable, for tenant is hard-coded to 'basic'. After that, additional auth mechanisms can be implemented as needed.

Do we consider FWD account's password hashed and stored in directory.xml secure?

We can have a functionality in Admin console to generate a token, that is stored in the DB. It can be used to ask /admin/login for auth token (that expires based on config). Something along those lines.

The token generated by the admin will not expire until deleted from the console and can be used in an admin app and it will be up to the customer to store it securely. In every case we'll need a new db table to store the login tokens. We can later build a more intricate authorization and associate permissions for each path, again stored in the db. I think we usually don't use db for FWD stuff, but I don't see it working without one, except if this API is not relying on 3rd party auth only.

Constantin Asofiei wrote:

The authentication method should be configurable, for tenant is hard-coded to 'basic'. After that, additional auth mechanisms can be implemented as needed.

I think we would migrate this to a better approach when the more general purpose REST API is implemented.

Galya B wrote:

The token generated by the admin will not expire until deleted from the console and can be used in an admin app and it will be up to the customer to store it securely. In every case we'll need a new db table to store the login tokens. We can later build a more intricate authorization and associate permissions for each path, again stored in the db. I think we usually don't use db for FWD stuff, but I don't see it working without one, except if this API is not relying on 3rd party auth only.

A persistent token would more naturally be stored/defined in the directory. We don't put any FWD server state or configuration into a customer's database. Although we have a landlord database for the tenant information, it is not intended to be widened to general purpose configuration or state.

A persistent token in directory will do exactly the same as the fwd pass.

I have another idea then: leave it to the customers completely. Ask them to implement an authenticator and pass in the authentication and authorization requests, leaving it up to them to handle credentials and permissions. Also having control over the headers and cookies.

As for GWT reusing the REST API, I think it's not necessary. It will require a major rework and will make development slower and the code messier or dependent on niche 3rd party libs. (this is a summary of my concerns without the explanation)

A persistent token in directory will do exactly the same as the fwd pass.

That is OK. There is no issue with storing sensitive information in the directory. It is full of sensitive information. Our threat model assumes that the directory is locked down in the file system and has extremely limited access. The server itself must have access, usually in a specific account like fwd. And of course, root has access. We already store hashed passwords, certificates and other authentication related state there.

I have another idea then: leave it to the customers completely. Ask them to implement an authenticator and pass in the authentication and authorization requests, leaving it up to them to handle credentials and permissions. Also having control over the headers and cookies.

We should not ask the customer to implement code in order for our administrative REST API to be functional. Our system should have functional administration built in.

As for GWT reusing the REST API, I think it's not necessary. It will require a major rework and will make development slower and the code messier or dependent on niche 3rd party libs. (this is a summary of my concerns without the explanation)

The REST API is happening regardless. If it really makes more sense to maintain a parallel RPC for GWT, that is OK, but it will come with its own costs.

Long term I think we will re-implement the admin console outside of GWT, going directly to the REST API. But in the short term, we will add REST and make that fully functional.

Greg Shah wrote:

A persistent token in directory will do exactly the same as the fwd pass.

That is OK. There is no issue with storing sensitive information in the directory. It is full of sensitive information. Our threat model assumes that the directory is locked down in the file system and has extremely limited access. The server itself must have access, usually in a specific account like fwd. And of course, root has access. We already store hashed passwords, certificates and other authentication related state there.

The storage place out of discussion, what is the benefit of maintaining a separate persistent token? Is there a reason to not reuse the fwd password? I would assume the admin user will be naturally selected to get the api permissions as well. As for the current approach with the webServiceToken, it's pretty much the same, except that the token is plain text in directory, but this seems to not be a major concern.

For API access, I would expect a cryptographically signed token to be issued (an "API key"). This has advantages over passwords or UUIDs/plain text tokens because such keys are not sensitive to brute force attacks. You still have to protect the key carefully but it cannot be easily forged.

In other words, I prefer not to use a basic auth approach for our admin API. Or at least we should have a slightly more secure approach available.

I believe you speak about the key that will come with each request. I was asking about the initial authentication.

Actually, this corresponds to your suggestion from #7080-26 except I would expect to store the key/token in the directory instead of a database. This kind of "shared secret" approach doesn't have an authentication step for each API flow.

We can later build a more intricate authorization and associate permissions for each path

FYI, we already have a full implementation of authorization/permissions for all paths in the directory.

Greg Shah wrote:

Actually, this corresponds to your suggestion from #7080-26 except I would expect to store the key/token in the directory instead of a database. This kind of "shared secret" approach doesn't have an authentication step for each API flow.

So you mean no shared secret (tokens), but authentication on each request?

Galya B wrote:

Greg Shah wrote:

Actually, this corresponds to your suggestion from #7080-26 except I would expect to store the key/token in the directory instead of a database. This kind of "shared secret" approach doesn't have an authentication step for each API flow.

So you mean no shared secret (tokens), but authentication on each request?

No, I mean that we can implement your idea from #7080-26 but the storage is in the directory, not a database. It is a shared secret approach and the admin can control the creation/deletion of the tokens. As noted above, I would prefer these to be signed so that they cannot be easily forged.

OK, let's call the tokens 'auth token' and 'request token' for clarity, because I get confused. So the admin has an admin console screen to create new encrypted auth tokens, set their permissions and delete them. This requires a pre-config of setting up a key for the encryption. The auth tokens are saved in directory. The server handles a login request with an auth token by decrypting the token and if it's signed by the key, the server returns the 'request token' that is a randomly generated string saved in the server memory for a configurable amount of time, and used for authentication/authorization of subsequent API calls. Via https the only benefit I see from using the 'request token' is saving on decrypting the auth token with each request, which is viable for heavy load APIs, maybe not so much for ours, although if we start providing CPU/mem usage for processes via the API the use may be high with some customers.

The auth aside, the GWT problem with reusing REST is that it's a framework that generates code both server-side and client-side, and facilitates the communication between both. We can't just get rid of half the framework functionality. The solutions out there, like resty-gwt, are just patching the rpc approach with adding more garbage in the middle. So in my opinion it's not worth it trying to patch it somehow, and as noted in #7080-32 it's best to stick with two independent APIs (RPC and REST), until time for the long term solution - the complete rework. That being said, is the rework going under this task, or is it to be considered closed?

Always the third option. :) OK, now speaking about REST API, can we consider doing it properly using swagger (having it in a declarative form, conforming to the OpenAPI specification) to be able to easily share it with the customers (and everyone developing)?

Yes, we can consider Swagger.

• /admin/sessions - SessionsHandler implementation for FWD session administration.
  

  /admin/sessions/login
  /admin/sessions/lock
  /admin/sessions/unlock
  /admin/sessions/all
  /admin/sessions/<session_id>
  

  • ACLs are prefixed with SESSION:
  • enabled via the server/default/sessions/enabled flag, and with specific timeout of the login tokens via server/default/sessions/enabled
• /admin/tenant/, for tenant related management. TenantHandler is the implementation.
  

  /admin/tenant
  /admin/tenant/login
  /admin/tenant/<tenant_name>
  /admin/tenant/<tenant_name>/status
  /admin/tenant/<tenant_name>/db
  

  • ACLs are prefixed with TENANT:
  • enabled via serfver/default/multi_tenancy/enabled flag, and with specific timeout of the login via serfver/default/multi_tenancy/timeout

In #8976 branch 8976b, TenantHandler was renamed to AdminHandler. This was set to a context path of /admin and after that 'manually' checks the target, to handle it. While this is acceptable for admin REST APIs grouped to support a certain feature like /admin/tenant or /admin/sessions, having a single /admin and after that using target.startsWith to identify the feature being handled is not manageable when we start adding new handlers for other features, like appserver administration, user administration, etc. The current AdminHandler code in 8976b, even if it works properly now, will not remain as it is - will be refactored anyway. So we need a decision what to do with 8976b - do we implement there the proper infrastructure for all REST admin APIs, or leave it for later?

Ovidiu - please document here all the REST paths added to AdminHandler in 8976b, in the way it was described above.

I think AdminHandler needs to be the 'base handler' which creates just a HandleCollection, with specific handlers for specific paths, only if their are enabled; this would make the StandardServer.registerDefaultServices agnostic to which REST admin features are enabled or not. Common behavior for each handler can be in a super-class (like for authentication & authorization).

• how to handle /login? A proposal would be this:
  • /admin/login would enable a login for all REST features (like a 'super-admin')
  • /admin/tenant/login would enable a login only for that feature, while not allowing access to others
• a better way to handle each REST feature being enabled/disabled and the token timeout (what we currently have in directory.xml for i.e. multi-tenancy and sessions).
• 8976b renames the ACL prefix from TENANT to REST-ADMIN, while leaving SESSION intact. IMO, we should keep granular prefixes, and allow the specific handler to specify its own prefix. Anyway, a decision needs to be made now.

Constantin Asofiei wrote:

Ovidiu - please document here all the REST paths added to AdminHandler in 8976b, in the way it was described above.

The new REST APIs are documented here. To summarize them, they are:

GET            /admin/login

GET            /admin/domain_type/login (synonym of the above)
GET/POST       /admin/domain_type/<database>
GET/PUT/DELETE /admin/domain_type/<database>/<domain-type>

GET            /admin/domain/login (synonym of the first)
GET/POST       /admin/domain/<database>
GET/PUT/DELETE /admin/domain/<database>/<domain-name>

Some other questions:

• how to handle /login? A proposal would be this:
  • /admin/login would enable a login for all REST features (like a 'super-admin')

This is how the AdminHandler does it now, doing the authentication for domain, domain_type, and tenant. As noted above, a synonym for each resource exists but, ultimately, the obtained token will work for all resources.

• /admin/tenant/login would enable a login only for that feature, while not allowing access to others

At least domain, domain_type, and tenant should go together. Maybe we can define some 'roles' which will allow specific resources to be administrated?

• a better way to handle each REST feature being enabled/disabled and the token timeout (what we currently have in directory.xml for i.e. multi-tenancy and sessions).

I am OK with current form, but if this can be improved, I am OK with it.

• 8976b renames the ACL prefix from TENANT to REST-ADMIN, while leaving SESSION intact. IMO, we should keep granular prefixes, and allow the specific handler to specify its own prefix. Anyway, a decision needs to be made now.

I had a quick discussion with Radu about this. As noted above, domain, domain_type, and tenant should be considered as a group and are handled together by the AdminHandler (the old TenantHandler, renamed). With current implementation, when this class is registered, a single 'token' can be used per handle, therefore I had to pick something common, more general than 'TENANT'. Making it more granular means more explicit for the ACL maintainer/admin but will require some mapping.

As a separate idea, the domain, domain_type, and tenant could use a different initial prefix, like /meta/, so we will have MetaHandler, /meta/tenant/, /meta/domain/, and /meta/domain_type/. There will be no collision with /admin/sessions. But this will only delay the resolution of the big issue.

/admin/login would enable a login for all REST features (like a 'super-admin')

Let's make this the only login for all admin APIs. One should not have to login to each admin service separately. So we would not have separate logins by service (/admin/tenant/login). Consider an obvious use case where an admin UI is written that provides features across multiple admin functional areas. We wouldn't want the users to have to login multiple times as they traverse various functions.

Our admin console will be rewritten that way. Customers may write their own admin UI or tools with the same needs.

As a separate idea, the domain, domain_type, and tenant could use a different initial prefix, like /meta/, so we will have MetaHandler, /meta/tenant/, /meta/domain/, and /meta/domain_type/. There will be no collision with /admin/sessions. But this will only delay the resolution of the big issue.

Let's keep things in /admimn/.

Ovidiu Maxiniuc wrote:

• a better way to handle each REST feature being enabled/disabled and the token timeout (what we currently have in directory.xml for i.e. multi-tenancy and sessions).

I am OK with current form, but if this can be improved, I am OK with it.

The part I don't like is the timeout - if we use only /admin/login, then the timeout should be set globally.

Greg, do we want to enable/disable certain rest ADMIN features? If not, then is enough to have a server/default/rest-admin/timeout node to set the timeout.

Ovidiu Maxiniuc wrote:

• 8976b renames the ACL prefix from TENANT to REST-ADMIN, while leaving SESSION intact. IMO, we should keep granular prefixes, and allow the specific handler to specify its own prefix. Anyway, a decision needs to be made now.

I had a quick discussion with Radu about this. As noted above, domain, domain_type, and tenant should be considered as a group and are handled together by the AdminHandler (the old TenantHandler, renamed). With current implementation, when this class is registered, a single 'token' can be used per handle, therefore I had to pick something common, more general than 'TENANT'. Making it more granular means more explicit for the ACL maintainer/admin but will require some mapping.

As a separate idea, the domain, domain_type, and tenant could use a different initial prefix, like /meta/, so we will have MetaHandler, /meta/tenant/, /meta/domain/, and /meta/domain_type/. There will be no collision with /admin/sessions. But this will only delay the resolution of the big issue.

You mean /admin/meta/tenant/? Then I'm OK to group these together, and use the META: prefix in the ACLs. And also rename AdminHandler to MetaHandler.

Greg: I was thinking about the prefix to be used in the ACLs, to either use ADMIN (or REST-ADMIN), or handler-specific prefixes. IMO, having the prefix to be specific to the handler will be more helpful in creating custom ACL UI in the Admin Console.

Constantin Asofiei wrote:

You mean /admin/meta/tenant/? Then I'm OK to group these together, and use the META: prefix in the ACLs. And also rename AdminHandler to MetaHandler.

I was not, but /admin/meta/<meta-tables>/ and MetaHandler will fit for me as a glove (or two of them? :)).

Greg: I was thinking about the prefix to be used in the ACLs, to either use ADMIN (or REST-ADMIN), or handler-specific prefixes. IMO, having the prefix to be specific to the handler will be more helpful in creating custom ACL UI in the Admin Console.

I subscribe to that, too.

Greg, do we want to enable/disable certain rest ADMIN features? If not, then is enough to have a server/default/rest-admin/timeout node to set the timeout.

No, it should be all or nothing.

Ovidiu Maxiniuc wrote:

Constantin Asofiei wrote:

You mean /admin/meta/tenant/? Then I'm OK to group these together, and use the META: prefix in the ACLs. And also rename AdminHandler to MetaHandler.

I was not, but /admin/meta/<meta-tables>/ and MetaHandler will fit for me as a glove (or two of them? :)).

The meta thing is confusing for me. I think of all the 4GL metadata (e.g. _file, _field) when I hear "meta" which is much broader than we would intend to implement. Also, while the domain stuff is metadata, the tenant stuff isn't as directly related.

My sense is that this obscures things, so I don't prefer it.

Greg: I was thinking about the prefix to be used in the ACLs, to either use ADMIN (or REST-ADMIN), or handler-specific prefixes.

Are you talking about the name of the security plugin that manages this resource?

Ultimately, these APIs will be THE administrative interface for FWD. We will eventually rework the current admin console to remove GWT and directly use the REST admin API.

IMO, having the prefix to be specific to the handler will be more helpful in creating custom ACL UI in the Admin Console.

If the relative URL paths are just to be treated as resource names, then they can have the whole /admin/what/ever and we get a simple way to differentiate the ACLs.

Greg Shah wrote:

Ovidiu Maxiniuc wrote:

I was not, but /admin/meta/<meta-tables>/ and MetaHandler will fit for me as a glove (or two of them? :)).

The meta thing is confusing for me. I think of all the 4GL metadata (e.g. _file, _field) when I hear "meta" which is much broader than we would intend to implement. Also, while the domain stuff is metadata, the tenant stuff isn't as directly related.
My sense is that this obscures things, so I don't prefer it.

Well, all these 3 sub-branches ultimately do the same: expose CRUD operations for the respective tables. Including the tenant. Even if the current JSON payload does not support the full record, it still operates on this table. This is how the customer expects is to work: add/update/remove a tenant using this REST API and the change will be visible in _Tenant meta table. However, this has an additional information to maintain: the private database configuration (url/user/password) which are not found in 4GL since we have other data architecture.

Not all meta tables will be exposed, only the information from REGULAR tables (meta tables which are moved to persistent storage in the primary database). The _User table also has this attribute so probably we will add the API for it, too. _File, _Field, _Index, etc are more like R/O VSTs and populated each time at server startup from the existing annotations.

I was not, but /admin/meta/<meta-tables>/ and MetaHandler will fit for me as a glove (or two of them? :)).

The meta thing is confusing for me. I think of all the 4GL metadata (e.g. _file, _field) when I hear "meta" which is much broader than we would intend to implement. Also, while the domain stuff is metadata, the tenant stuff isn't as directly related.
My sense is that this obscures things, so I don't prefer it.

Well, all these 3 sub-branches ultimately do the same: expose CRUD operations for the respective tables. Including the tenant. Even if the current JSON payload does not support the full record, it still operates on this table. This is how the customer expects is to work: add/update/remove a tenant using this REST API and the change will be visible in _Tenant meta table. However, this has an additional information to maintain: the private database configuration (url/user/password) which are not found in 4GL since we have other data architecture.

And also the landlord database is managed by this API. My point here: it is not the same thing as metadata and I don't want people to think that it is such a thing.

Greg, there is currently a single handler for /admin/tenant, /admin/domain and /admin/domain-type. The last 2 I think are for meta database, and should be in the (new) MetaHandler. Is this what you mean?

Greg, there is currently a single handler for /admin/tenant, /admin/domain and /admin/domain-type. The last 2 I think are for meta database, and should be in the (new) MetaHandler. Is this what you mean?

I mean that I want to keep the current categories of admin APIs. I don't want to use the terminology of metadata or meta here because I think it will cause issues with customer's understanding of this support. These APIs are exclusively for administration. It is not general purpose metadata support, even if it does overlap with those tables.

As far as the handlers are concerned, I don't care too much. Use an implementation that is easy to understand and support while meeting the core objectives of our overall admin API.

Greg Shah wrote:

Greg, there is currently a single handler for /admin/tenant, /admin/domain and /admin/domain-type. The last 2 I think are for meta database, and should be in the (new) MetaHandler. Is this what you mean?

I mean that I want to keep the current categories of admin APIs. I don't want to use the terminology of metadata or meta here because I think it will cause issues with customer's understanding of this support. These APIs are exclusively for administration. It is not general purpose metadata support, even if it does overlap with those tables.

OK, so my thinking is to have a handler for all APIs which have a common behavior. Also, this would translate into 'the paths used by a certain ACL prefix' like SESSION are handled by a single handler (although there's nothing stopping to have another handler setup with the same ACL prefix, but a handler would not be able to manage multiple ACL prefixes).

Ovidiu, the domain and domain-type, are these only related to multi-tenancy? Because as I recall these can also be involved with CLIENT-PRINCIPAL and this doesn't have any dependency on multi-tenancy.

Ovidiu, the domain and domain-type, are these only related to multi-tenancy? Because as I recall these can also be involved with CLIENT-PRINCIPAL and this doesn't have any dependency on multi-tenancy.

That is my understanding as well. These are not only used for multi-tenant.

The domain table has references to domain_type and tenant_name. Therefore a domain cannot be defined without having a record in these tables.

OTOH, I think CLIENT-PRINCIPAL uses the Default tenant / domain as default if none is used.

Greg, currently the _sec-authentication-system and _sec-authentication-domain are 'written' via TenantManager, but otherwise 'read' from SecurityOps. I'm not sure this is OK, I understand that these are (mostly) used for tenant management, they are not necessarily dedicated for multi-tenancy.

I would suggest to keep this management separated in SecurityOps, and also the REST APIs in a different handle and REST paths. Because we shouldn't make it look like you need multi-tenancy enabled to manage these tables.

Yes, I agree. Those tables are security-related, not tenant-related.

Another note here: I think we need the REST admin APIs to have mandatory authentication/authorization.

Ovidiu: please let me know if you have questions, we can discuss.

Constantin Asofiei wrote:

Another note here: I think we need the REST admin APIs to have mandatory authentication/authorization.

The admin console (web application) is supposed to use this REST API in the future. So please consider web standards before committing to a long-term solution for the authentication/authorization.

Idea discussed in the daily meeting today:

• As this task is also about (re-)designing the REST API to expose the Admin API, I was suggesting having the versioning embedded in the REST endpoints like:
  

  /admin/v1/sessions/login
  /admin/v1/sessions/lock
  /admin/v1/sessions/unlock
  /admin/v1/sessions/all
  /admin/v1/sessions/<session_id>

• having it after/before the admin particle are both fine.
• this idea is not necessarily strongly bound to admin console .. maybe other interfaces we have by REST can be included in the discussion.

My point is that the REST API seem to be changing (starting with this task) and FWD may not presumably stop changing the REST API in the future. Having it versioned might provide more stability to applications that use the FWD infrastructure and want to upgrade to later versions without revamping their other services that interact with FWD.

For this very specific task, having what is already in FWD trunk as v0 and building a v1 from scratch may make sense. Of course, some endpoints in v1/ might simply delegate to v0/, aggregate some v0/ calls, etc.. The point is that customers already using admin console will have their integrations broken after upgrade when #7080 will be merged. Is that the case? maybe/maybe not. But in the future this scenario may become more and more probable IMHO. A future v2/ would benefit from such early decision.

I am writing this at the task to ask of Greg's feedback on the idea and whether it should be implemented in this stage of development for the REST API.

Versioning is introduced when the team is on the same page about the definition of backwards compatibility. Breaking changes up the version number. A breaking change is to remove a property from a json result. Or introduce authentication and authorization. In other words v1 suggests the standardization of the API is already confirmed and no breaking changes are to be introduced ever in that version.

A question I have is do we expect the framework to ever expose endpoints that are not /admin, because to me everything you can do in a framework goes under admin, and it kind of looks like this is the direction it evolves already.

I'm not opposed to an explicit versioning in the URL, but this also would suggest that the older versions would be supported which is not necessarily correct.

If we went down this path:

• The version would probably be after sub-api level (/admin/sessions/v1/ instead of /admin/v1/sessions/). The point here is that different sub-apis would probably have different versions and would not be kept at a single top-level version. If we did it at the top level, then we would probably want to batch version changes otherwise we might have a v243 before long and I don't think that is helpful.
• I would wait until we are further along in our development. For now, we have so few users and the work is so fluid that I would not assume that the APIs are very fixed yet.

A question I have is do we expect the framework to ever expose endpoints that are not /admin, because to me everything you can do in a framework goes under admin, and it kind of looks like this is the direction it evolves already.

All administrative endpoints will be in /admin/ and we will group them by "sub-api" like sessions. It reads well, keeps the admin API together and yet allows like-subsets to be grouped as well.

Constantin and I have a discussion on meet and we reach the following conclusions (summarised):

1. instead a having N handlers 'externally' instantiated in StandardServer, a new AdminRestHandler (which will extend HandlerCollection) will be implemented which will allow the specialised handlers to load/initialise based on their own specific criteria. This will be the only handler registered to StandardServer;
2. the AdminRestHandler will have the basepath /admin and directly handle /login and /logout, making the login/authentication common for all resources;
3. the resource-specific handlers (e.g. SessionHandler for context path /admin/sessions) will have the access to authentication token so they can be authorised;
   • the ACLs will be used for fine-tuning the access for these handlers.
   • question: Should a handler manage multiple ACL prefixes? (no, see bellow, ADMIN will be the only ACL prefix)
4. configuration
   • the individual settings will be replaced by single directory configuration, like an admin-rest node, which will include:
     • enabled/disabled: this will disable or enable all the REST admin handlers - there is no handler-specific setting here. Question: is this the same as adminEnabled in directory?

           <node class="container" name="admin-rest">
               <node class="boolean" name="enabled">
                 <node-attribute name="value" value="TRUE"/>
               </node>
               <node class="integer" name="timeout">
                 <node-attribute name="value" value="30"/>
               </node>
           </node>

     • authorisation of a specific user to certain REST APIs will be done via ACLs;
   • ACL prefixes:
     • all handlers will use ADMIN as the ACL prefix for web security (new, to be implemented!);
     • a super-admin must be able to configure a ADMIN:*:* ACL to allow access to everything (new, to be implemented!);
     • specific ACLs can be grouped together by the ‘context path’, which will be common to admin features which are related (like lets say a handler for users/groups will be under /admin/security/);
   • API definition for the REST resources
     • Authentication:
       • as noted above, /admin/login and /admin/logout endpoints.
       • Login sets a FwdSessionId header in response (unchanged). Only basic authentication is supported at this time, using a FWD username and FWD user ‘web token’ as password;
     • Authorisation: each REST request will send a FwdSessionId header with the token retrieved from authentication (unchanged)
     • Current handlers: Sessions: /admin/sessions, Tenant: /admin/tenants, Security: /admin/security/ for the domain and system security meta tables (for now). The latest will have to be remapped from /admin/domain to /admin/security/domains, etc
5. change proposals. To meet the REST specifications:
   • the lock/unlock operations of Sessions handler should be moved to PUT method instead of GET;
   • the /admin/tenant should be mapped to /admin/tenants;
   • the /admin/tenants and /admin/sessions should return only the list of resources, not the full details. The individual details are provided by /admin/tenants/<id> and /admin/sessions/<id>

The critical is to have the configuration and prrotocol stable so that the customers will have a clear API to work with.

There are several changes to implement and other to add in this document. I expect 2-3 DoW to get it done.

I agree with these ideas.

The only difference is that I think we need to move away from basic auth and design a more appropriate mechanism. We should at least design the concept now even if we defer implementation.

Ovidiu Maxiniuc wrote:

• the /admin/tenants and /admin/sessions should return only the list of resources, not the full details. The individual details are provided by /admin/tenants/<id> and /admin/sessions/<id>

• what will GET:/admin/sessions/ return? The session IDs? This tells nothing to the user; neither the FWD user id, maybe the OS user id will be used.
• when we'll move the FWD Admin Console to a RESTfull app, how many requests will we need to i.e. populate a table with more than the tenant name?

The point is: I see real usage in getting the full details 'up front', and not having to fetch them one by one.

Constantin Asofiei wrote:

The point is: I see real usage in getting the full details 'up front', and not having to fetch them one by one.

In these cases, the responses are small, at least within our testing environments. But as the database gets more populated, the response might get overwhelming.
So I think we can keep the full lists, but we seriously need to think about filtering / pagination, so only a specific subset of the full list is processed by both the server and the client.

Greg Shah wrote:

The only difference is that I think we need to move away from basic auth and design a more appropriate mechanism. We should at least design the concept now even if we defer implementation.

Supporting a web standard authentication is a requirement for #9519. RFC 7519 (JWT) is exactly that. Long-lived tokens are not appropriate for web, so JWT offers digital signing, refresh tokens and short-lived access tokens. JWT is usually stored as a cookie in the web browser, so to not interfere with the customer apps it's best to be implemented together with #9405. If the customers need long-lived tokens for their tools we should somehow enable them only for local use (from the same network) and consider providing a token revocation mechanism. Also, a consideration would be if the tokens should be persisted between server restarts.

It's a good practice the auth endpoints to have their own path, i.e. /admin/v1/auth/login, /admin/v1/auth/refresh, /admin/v1/auth/logout.

I think we should also consider generating the api interface and documentation pages with Swagger before we're too deep into the development. Some work put initially saves a lot of time, efforts and troubles after that. Conforming to OpenAPI can speed up development of the admin console when the SDK gets auto-generated.

Greg Shah wrote:

Yes, we can consider Swagger.

Constantin, does #7080-80 mean we're not considering Swagger for server stubs?

Galya B wrote:

Greg Shah wrote:

Yes, we can consider Swagger.

Constantin, does #7080-80 mean we're not considering Swagger for server stubs?

How does Swagger generate the server stubs? With annotations? If so, then how would this be ran inside the FWD server - what control do we have of starting this in the same embedded jetty process as the other handlers we have, like /gui, /chui, /soap, etc? Because this is a pretty important implementation detail.

Constantin Asofiei wrote:

How does Swagger generate the server stubs? With annotations?

The api is declared in yaml or json file: all endpoints, filters, exceptions, results (type, field name, etc.), auth methods, endpoint description (javadoc). Then you generate the stubs of any popular technology you like to use, I would recommend Java Jersey 2 (JAX-RS), that is the go-to plain Java. This will be added to the step ant-jar. It generates Java classes in a specified dir that is ignored from the vcs. This way the interface is simply a declarative file. The generated files are always up-to-date with the javadoc and the SDKs for the clients because all are generated from the same definition, and all of this is technology and language agnostic, saves tons of bugs, debugging and supporting separate wikis.

This approach is the architect's approach: instead of coding and then deciding on the interface contract, you first start with the API design. Then wire up the interface to the actual implementation that are services.

Jersey itself uses annotations to expose endpoints, but the source file not.

If so, then how would this be ran inside the FWD server - what control do we have of starting this in the same embedded jetty process as the other handlers we have, like /gui, /chui, /soap, etc? Because this is a pretty important implementation detail.

The declarative api you create has all the details of endpoint paths, you have full control of what is exposed. Also we should consider implementing #9405 with this task. Jersey can be configured to work with a specific embedded server, but this is not something I can pull out of my back pocket right away. In any case it's a matter of configuration.

Here is a full list of the supported server stubs, client SDKs and doc types it can generate: swagger-codegen. Another nice integration it has is with JMeter, the endpoints can be easily imported in JMeter, which imo is a good tool to share tests in the team, since it's free and has a lot of features.

Galya B wrote:

In any case it's a matter of configuration.

https://anirtek.github.io/java/jetty/swagger/openapi/2021/06/12/Hooking-up-OpenAPI-with-Jetty.html

If the customers don't want to develop tools (API clients), they can have the auto-generated always up-to-date web client for the API, example in https://generator3.swagger.io/.

Greg: before committing to use Swagger and Jersey, I'd like to have a simple POC to check how it integrates with FWD.

Ovidiu: I like the way your work is moving. The design in #7080-80 is good.
Constantin: We should go ahead with a code review now.

In regard to Swagger, now is a good time to consider this. I would prefer to take that approach so long as it doesn't compromise our critical requirements. My thoughts:

• JSON or Annotations
  • It seems that we can either define the API in JSON and then generate Java stubs for it OR we can define the API as JAX-RS annotations and then generate the JSON from that.
  • I prefer to define the API in JAX-RS annotations and then generate the OpenAPI json from that. The reason is simple: the API definition is kept close to the code that implements it.
• I don't see the benefit of writing this as json and then generating server stubs. The OpenAPI json is nice for clients and for generating an SDK. We can still do that without writing the API itself in JSON. Going the other way around is messy because we need to match our coding standards and over time we have to reintegrate any changes in generated stubs into the Java code that already exists. That doesn't seem like a good process.
• One thing I'm worried about is exposing this dynamic API documentation on a production system. Not all customers will be OK with that. This has nothing to do with the knowledge of the API being public. The API will always be public knowledge. The issue is that running extra code to dynamically generate and expose the API increases the attack surface of the production system, without any real value. Development against the API will be done in a dev lab not on production systems. I would expect us to make this configurable.
• We already use Jackson for data binding with json. To the extent that such a technique will be included in our solution, I would expect to use Jackson unless there is a strong reason otherwise. It can be made to work with Jersey. If we don't need it, that is also fine.

Greg Shah wrote:

• JSON or Annotations
  • It seems that we can either define the API in JSON and then generate Java stubs for it OR we can define the API as JAX-RS annotations and then generate the JSON from that.

Actually the usual choice is yaml because it's shorter, more clear format.

• I prefer to define the API in JAX-RS annotations and then generate the OpenAPI json from that. The reason is simple: the API definition is kept close to the code that implements it.

This is the legacy approach, when you want to modernize an old API and can't afford to start clean.

• I don't see the benefit of writing this as json and then generating server stubs. The OpenAPI json is nice for clients and for generating an SDK. We can still do that without writing the API itself in JSON.

YAML please.

Going the other way around is messy because we need to match our coding standards and over time we have to reintegrate any changes in generated stubs into the Java code that already exists. That doesn't seem like a good process.

You can generate only the interfaces and model classes and not regenerate the actual service classes where the actual implementation is. These regenerated files are out of the source files anyways.

• One thing I'm worried about is exposing this dynamic API documentation on a production system. Not all customers will be OK with that.

I thought I mentioned many times #9405, so the doc + the Admin console will have the same accessability. AND obviously you can disable generating the web client for the API. You can still generate it locally when developing and use it in test environments.

This has nothing to do with the knowledge of the API being public. The API will always be public knowledge. The issue is that running extra code to dynamically generate and expose the API increases the attack surface of the production system, without any real value. Development against the API will be done in a dev lab not on production systems. I would expect us to make this configurable.

It is configurable of course. Didn't you ask me to create #9405 to decrease the attack surface? What is so stressful here I don't get.

• We already use Jackson for data binding with json. To the extent that such a technique will be included in our solution, I would expect to use Jackson unless there is a strong reason otherwise. It can be made to work with Jersey. If we don't need it, that is also fine.

Jackson is used by default by the stub generator.

These are standards adopted by the corporations for many years now, including banks. But if fear of improvement is the primary drive this won't fix the reoccurring issues in FWD with manual errors and slow dev time and not enough testing.

Greg Shah wrote:

The issue is that running extra code to dynamically generate and expose the API increases the attack surface of the production system, without any real value.

You can see with your own eyes the generated java classes before archiving - core Java, Jackson and Jersey. The swagger libs are needed only compile time, not runtime. They can be even used as external tools instead of compile dependencies. The web client is disabled in prod (but in our case if the Admin Console and the admin API are available only in a local network for prod I don't see a problem to have the web client there too). As for the real value of automation... I don't want to get there.

• JSON or Annotations
  • It seems that we can either define the API in JSON and then generate Java stubs for it OR we can define the API as JAX-RS annotations and then generate the JSON from that.

Actually the usual choice is yaml because it's shorter, more clear format.

Sorry, I have to disagree with you there. YAML is a hot mess. It allows all kind of nastiness that has zero value.

• I prefer to define the API in JAX-RS annotations and then generate the OpenAPI json from that. The reason is simple: the API definition is kept close to the code that implements it.

This is the legacy approach, when you want to modernize an old API and can't afford to start clean.

Just saying "it's legacy" doesn't make it the wrong way to go. The only advantage I see in coding the API in JSON/YAML is if you were implementing that API in multiple languages, so you might get a slight benefit to doing some stub generation. We don't have that requirement. Do you have some value proposition that explains why we would want to separate our API from our code? I don't see the benefit.

• I don't see the benefit of writing this as json and then generating server stubs. The OpenAPI json is nice for clients and for generating an SDK. We can still do that without writing the API itself in JSON.

YAML please.

If we generate the OpenAPI docs from our annotations, then you can have your wonderful YAML as well as JSON.

• One thing I'm worried about is exposing this dynamic API documentation on a production system. Not all customers will be OK with that.

I thought I mentioned many times #9405, so the doc + the Admin console will have the same accessability. AND obviously you can disable generating the web client for the API. You can still generate it locally when developing and use it in test environments.

This has nothing to do with the knowledge of the API being public. The API will always be public knowledge. The issue is that running extra code to dynamically generate and expose the API increases the attack surface of the production system, without any real value. Development against the API will be done in a dev lab not on production systems. I would expect us to make this configurable.

It is configurable of course. Didn't you ask me to create #9405 to decrease the attack surface? What is so stressful here I don't get.

The API is defined at development time, not at runtime. You don't need to dynamically generate it on a production server. The JSON/YAML can be generated at build time and then we can expose those static documents however we want.

These are standards adopted by the corporations for many years now, including banks.

Banks don't build platforms, they build applications. FWD is not an application, it is a platform on which applications are built. We must think things through that organizations like Banks can take for granted. We will have a discussion about things like this. We will discuss and the pros and cons of things. Then we will decide what is best for our use case.

As for the real value of automation... I don't want to get there.

I never said that. This has nothing to do with automation.

Constantin Asofiei wrote:

Greg: before committing to use Swagger and Jersey, I'd like to have a simple POC to check how it integrates with FWD.

Agreed.

Greg Shah wrote:

As for the real value of automation... I don't want to get there.

I never said that. This has nothing to do with automation.

Generating huge amounts of bug-free code automatically is automation to me. I think in FWD we have this notion that automation is not necessarily running on a dedicated CI/CD machine, so my reading of the term doesn't contradict it.

Greg Shah wrote:

• JSON or Annotations
  • It seems that we can either define the API in JSON and then generate Java stubs for it OR we can define the API as JAX-RS annotations and then generate the JSON from that.

Actually the usual choice is yaml because it's shorter, more clear format.

Sorry, I have to disagree with you there. YAML is a hot mess. It allows all kind of nastiness that has zero value.

The nastiness is to make it machine readable, but short. The shortness is something we don't value here, but usually the styling of code (default linter settings) requires the reader to be able to understand quickly the content without scrolling like mad.

• I prefer to define the API in JAX-RS annotations and then generate the OpenAPI json from that. The reason is simple: the API definition is kept close to the code that implements it.

This is the legacy approach, when you want to modernize an old API and can't afford to start clean.

Just saying "it's legacy" doesn't make it the wrong way to go. The only advantage I see in coding the API in JSON/YAML is if you were implementing that API in multiple languages, so you might get a slight benefit to doing some stub generation. We don't have that requirement. Do you have some value proposition that explains why we would want to separate our API from our code? I don't see the benefit.

The benefit is less developers' hands will be touching the code, less developers will simulate spending time on copy-pasting boilerplate code, less development time, easier tracking of changes, because all of them are in one single file in the version control, and not missing something to see a breaking change during code review. Also applying standards is easier in that one file that can even have linters running over it, instead of going over multiple boilerplate files. Although what we do here usually is place everything in one gigantic file, so some of my arguments may not be valid. Mix http logic into business logic into model...

• I don't see the benefit of writing this as json and then generating server stubs. The OpenAPI json is nice for clients and for generating an SDK. We can still do that without writing the API itself in JSON.

YAML please.

If we generate the OpenAPI docs from our annotations, then you can have your wonderful YAML as well as JSON.

Well, this is the only thing that is left out of it.

• One thing I'm worried about is exposing this dynamic API documentation on a production system. Not all customers will be OK with that.

I thought I mentioned many times #9405, so the doc + the Admin console will have the same accessability. AND obviously you can disable generating the web client for the API. You can still generate it locally when developing and use it in test environments.

This has nothing to do with the knowledge of the API being public. The API will always be public knowledge. The issue is that running extra code to dynamically generate and expose the API increases the attack surface of the production system, without any real value. Development against the API will be done in a dev lab not on production systems. I would expect us to make this configurable.

It is configurable of course. Didn't you ask me to create #9405 to decrease the attack surface? What is so stressful here I don't get.

The API is defined at development time, not at runtime. You don't need to dynamically generate it on a production server. The JSON/YAML can be generated at build time and then we can expose those static documents however we want.

I don't understand where is the contradiction, this kind of sounds like a contradiction of your own initial remarks. I've never spoken (never seen and never a thought has gone through my mind) that dynamic generation of api is going to end up in a prod server.

These are standards adopted by the corporations for many years now, including banks.

Banks don't build platforms, they build applications. FWD is not an application, it is a platform on which applications are built. We must think things through that organizations like Banks can take for granted. We will have a discussion about things like this. We will discuss and the pros and cons of things. Then we will decide what is best for our use case.

Sure, now that I've stirred things up discuss them. Otherwise pretend comments don't exist.

Greg Shah wrote:

Banks don't build platforms, they build applications. FWD is not an application, it is a platform on which applications are built. We must think things through that organizations like Banks can take for granted.

I was speaking about security and impact on revenue. I don't know if there are many other organizations worried about these to such extend, probably only NASA and FBI.

Galya B wrote:

Greg Shah wrote:

As for the real value of automation... I don't want to get there.

I never said that. This has nothing to do with automation.

Generating huge amounts of bug-free code automatically is automation to me.

I don't see the time savings here. Either way you have to define the API. Defining the API in JSON or YAML is no less a coding exercise than creating methods and writing annotations in Java. The syntax is different. Since we are Java developers, why would we want to define our APIs in JSON or YAML? This definition isn't really data at all. It is code in a weird format that you then need to execute a transformation step to turn it into Java code. Which we then have to implement separately.

The real effort in any API is in the design steps, which we are already doing. Since this is a Java project, coding the API as Java is more natural than coding it in JSON or YAML. We can easily generate the JSON and YAML for the consumers of the API. That seems to be the core of the value proposition here.

I think in FWD we have this notion that automation is not necessarily running on a dedicated CI/CD machine, so my reading of the term doesn't contradict it.

Sorry, I don't understand what CI/CD (which we are in fact implementing) has to do with designing and implementing an API.

The API is defined at development time, not at runtime. You don't need to dynamically generate it on a production server. The JSON/YAML can be generated at build time and then we can expose those static documents however we want.

I don't understand where is the contradiction, this kind of sounds like a contradiction of your own initial remarks. I've never spoken (never seen and never a thought has gone through my mind) that dynamic generation of api is going to end up in a prod server.

This link you posted (https://anirtek.github.io/java/jetty/swagger/openapi/2021/06/12/Hooking-up-OpenAPI-with-Jetty.html) has a step 3 that seems to implement a dynamic generation of the JSON/YAML. If I misunderstood that, then great.

My point is that we won't unconditionally have that feature.

Greg Shah wrote:

The API is defined at development time, not at runtime. You don't need to dynamically generate it on a production server. The JSON/YAML can be generated at build time and then we can expose those static documents however we want.

I don't understand where is the contradiction, this kind of sounds like a contradiction of your own initial remarks. I've never spoken (never seen and never a thought has gone through my mind) that dynamic generation of api is going to end up in a prod server.

This link you posted (https://anirtek.github.io/java/jetty/swagger/openapi/2021/06/12/Hooking-up-OpenAPI-with-Jetty.html) has a step 3 that seems to implement a dynamic generation of the JSON/YAML. If I misunderstood that, then great.

My point is that we won't unconditionally have that feature.

The guide is far from close to what needs to be integrated into our scripts / builds in FWD, it was linked only to show configs for enabling jetty work with Jersey. The same goes without saying when it comes to the package com.example.sample-package, or dependency lib versions...

Greg Shah wrote:

Galya B wrote:

I think in FWD we have this notion that automation is not necessarily running on a dedicated CI/CD machine, so my reading of the term doesn't contradict it.

Sorry, I don't understand what CI/CD (which we are in fact implementing) has to do with designing and implementing an API.

You often use the word automation in regards to manually starting scripts / commands / tests, when CI/CD has nothing to do with them. So I'm saying it's automation and CI/CD has nothing to do with it. It's the same FWD "run something, get something out of it" automation.

Greg Shah wrote:

Galya B wrote:

Greg Shah wrote:

As for the real value of automation... I don't want to get there.

I never said that. This has nothing to do with automation.

Generating huge amounts of bug-free code automatically is automation to me.

I don't see the time savings here. Either way you have to define the API. Defining the API in JSON or YAML is no less a coding exercise than creating methods and writing annotations in Java. The syntax is different. Since we are Java developers, why would we want to define our APIs in JSON or YAML? This definition isn't really data at all. It is code in a weird format that you then need to execute a transformation step to turn it into Java code. Which we then have to implement separately.

The real effort in any API is in the design steps, which we are already doing. Since this is a Java project, coding the API as Java is more natural than coding it in JSON or YAML. We can easily generate the JSON and YAML for the consumers of the API. That seems to be the core of the value proposition here.

If you don't create proper source files, good luck with generating OpenAPI declaration out of it. I haven't seen good separation of concerns in this project so far, but we can step up our game suddenly. I've mentioned a few more benefits, obviously not priorities to be completely dismissed as non-existing. Declarative forms are considered superb for obvious reasons, which don't seem to be so obvious here. People nowadays bring up whole networks from declarative files, but it doesn't seem reasonable for a mere api.

• please run javadoc with Java 8 and fix any issues in your files, if you haven't already
• TenantRestHandler
  • missing class javadoc
• AdminRestHandler
  • authHandler = new WebAuthHandler("admin", "basic", "/admin/login", "/admin/logout", timeout); needs to have REST-ADMIN as first argument (for the ACLs)
• in initialize(WebAuthHandler ah) for the sub-classes, you return the direct handler. Why not return a ContextHandler which adds this handler, and also sets the context path to i.e. /admin/security/domains? This will make the implementation cleaner for sub-classes, and in BaseAdminRestHandler.
  • there is a System.err.println("AdminRestHandler:handle(" + target + ")"); line - please check for others
• TenantManager - there is a // TODO: delete tenant from _Tenant
• WebServiceResource - "SESSION".equals(service) is no longer needed
• I have not given an in-depth look at the persistent changes

Also, please try to document the REST APIs in Swagger, and lets see how they look.

Constantin Asofiei wrote:

I have not given an in-depth look at the persistent changes

Code review 8976a/15809-15810 (persistence only):

There is not much change to persistence in these revisions, but the changes that are there look ok to me.

In many files, there is a change (in some cases it's the only change) to add a Redmine task reference number to the file header. I prefer we ultimately remove these.

Constantin Asofiei wrote:

Review of 8976a rev 15810:

• please run javadoc with Java 8 and fix any issues in your files, if you haven't already

The compatibility issues with JDK8 were addressed in r15811. There are no javadoc issues related to affected files.

• TenantRestHandler: missing class javadoc

Thanks, added it. Will be part of next commit.

• AdminRestHandler: authHandler = new WebAuthHandler("admin", "basic", "/admin/login", "/admin/logout", timeout); needs to have REST-ADMIN as first argument (for the ACLs)

I thought we decided to have the prefix just ADMIN:. See #7080-74.

• in initialize(WebAuthHandler ah) for the sub-classes, you return the direct handler. Why not return a ContextHandler which adds this handler, and also sets the context path to i.e. /admin/security/domains? This will make the implementation cleaner for sub-classes, and in BaseAdminRestHandler.

I tested this, as it seems the 'provided' way of doing things. Comparing the two approaches, the ContextHandler seems to add things, beside the extra object, a 5 level execution stack. The only gain is that the line newTarget = originalTarget.substring(contextPath.length()); is executed as target = target.substring(this._contextPath.length()); somewhere along this extra stack entries.

• there is a System.err.println("AdminRestHandler:handle(" + target + ")"); line - please check for others

I forgot about that. Removed. It should be the only one.

• TenantManager - there is a // TODO: delete tenant from _Tenant

Removed TODO, but kept the rest. The tenant is deleted by the listener implementer

• WebServiceResource - "SESSION".equals(service) is no longer needed

I have a couple of questions here. This code is not actually invoked. I put a breakpoint here and it is not reached. Also I can confirm that in my directory, all ACLs are suffixed by ADMIN: (see above), not REST-ADMIN as it is tested in the previous line. So the questions are when is this class instantiated (I could not find any constructor called) and when is the isInstanceNameValid() invoked? Is the class really needed?

Also, please try to document the REST APIs in Swagger, and lets see how they look.

I will, although I do not know how it will identify the implementing classes/methods in our project. There are no annotations, at least that I am aware of. Either way, I will post the result.

Ovidiu Maxiniuc wrote:

• AdminRestHandler: authHandler = new WebAuthHandler("admin", "basic", "/admin/login", "/admin/logout", timeout); needs to have REST-ADMIN as first argument (for the ACLs)

I thought we decided to have the prefix just ADMIN:. See #7080-74.

You have src/com/goldencode/p2j/security/WebServiceResource.java:150: "REST-ADMIN".equals(service) || - so you need to use ADMIN in both.

• in initialize(WebAuthHandler ah) for the sub-classes, you return the direct handler. Why not return a ContextHandler which adds this handler, and also sets the context path to i.e. /admin/security/domains? This will make the implementation cleaner for sub-classes, and in BaseAdminRestHandler.

I tested this, as it seems the 'provided' way of doing things. Comparing the two approaches, the ContextHandler seems to add things, beside the extra object, a 5 level execution stack. The only gain is that the line newTarget = originalTarget.substring(contextPath.length()); is executed as target = target.substring(this._contextPath.length()); somewhere along this extra stack entries.

OK, then I don't complain. The only issue is if the original target path is like /admin/////tenant///p1////p2///. We need to ensure the path is normalized.

• WebServiceResource - "SESSION".equals(service) is no longer needed

I have a couple of questions here. This code is not actually invoked. I put a breakpoint here and it is not reached. Also I can confirm that in my directory, all ACLs are suffixed by ADMIN: (see above), not REST-ADMIN as it is tested in the previous line. So the questions are when is this class instantiated (I could not find any constructor called) and when is the isInstanceNameValid() invoked? Is the class really needed?

Have you configured WebServiceResource resource in directory.xml, under resource-plugins? If is missing, then ACLs are not checked.

Also, please try to document the REST APIs in Swagger, and lets see how they look.

I will, although I do not know how it will identify the implementing classes/methods in our project. There are no annotations, at least that I am aware of. Either way, I will post the result.

I'm more interested to have a high-level view of the APIs in a single place, and as secondary what the generated code would look like.

Constantin Asofiei wrote:

Ovidiu Maxiniuc wrote:
You have src/com/goldencode/p2j/security/WebServiceResource.java:150: "REST-ADMIN".equals(service) || - so you need to use ADMIN in both.

OK.

[...] The only issue is if the original target path is like /admin/////tenant///p1////p2///. We need to ensure the path is normalized.

Actually, there is a property which forces the slashes to be merged (using URIUtil.compactPath()). It's name is compactPath. By default it is false therefore paths as above will not be normalised, therefore NOT correctly dispatched.

Have you configured WebServiceResource resource in directory.xml, under resource-plugins? If is missing, then ACLs are not checked.

In /security/config/resource-plugins I have this:

         <node-attribute name="values" value="com.goldencode.p2j.security.WebServiceResource"/>

Also, please try to document the REST APIs in Swagger, and lets see how they look.

I will, although I do not know how it will identify the implementing classes/methods in our project. There are no annotations, at least that I am aware of. Either way, I will post the result.

I'm more interested to have a high-level view of the APIs in a single place, and as secondary what the generated code would look like.

I am pretty new to this. I understand that we need to add annotations like @RestController, @Tag(name = "name"), @PostMapping("/<path>") and then use some tool to extract them in a manner similar to javadoc. Or maybe better to compare it to our report utility? If this is a Spring background worker, I will agree with Greg that this is an undesired overhead.

Ovidiu Maxiniuc wrote:

Have you configured WebServiceResource resource in directory.xml, under resource-plugins? If is missing, then ACLs are not checked.

In /security/config/resource-plugins I have this:[...]But it does not matter the code in WebServiceResource. It is not executed. I do not know the reason. A bug maybe?

Are the ACLs honored by authorize? I don't recall exactly, but looks like this code in isInstanceNameValid is executed from the FWD Admin Console, when you add/edit the ACLs. In any case, this needs to be 'in sync'.

I am pretty new to this. I understand that we need to add annotations like @RestController, @Tag(name = "name"), @PostMapping("/<path>") and then use some tool to extract them in a manner similar to javadoc. Or maybe better to compare it to our report utility? If this is a Spring background worker, I will agree with Greg that this is an undesired overhead.

I was referring 'the other way around', for now, to check what Swagger can define - see https://editor.swagger.io/ - and how the generated code looks like. Although our approach probably will be to write the JAX-RS annotations and from that generate the Swagger file, I'm more interested how the JAX-RS annotated .java code used as input by Swagger, would look like.

Anyway, I've found what I wanted for now, so please don't work on this now.

OTOH, there is always a 'better way' to do things, I don't recall why I didn't add a filter for the authorization. I don't think we should do this now, but something to keep in mind.

Ovidiu, I see that you changed SessionsRestHandler to use DELETE:/admin/sessions/all instead of DELETE:/admin/sessions/all to terminate all sessions. Please change it back. Otherwise I'm OK with the changes in 8976a/15827.

• make sure the ACLs are honored by authorize
• double-check the Wikis for tenant, session, etc that the REST APIs documented there are still in sync.
• update the wiki with the directory configuration (and ACLs, security, enabled, etc) to match what is in 8976a. Probably we need a new wiki page to describe this common configuration, and from that link to specific wikis for specific resources (tenant, sessions, etc).

Ovidiu, please go through the points in #7080-108, if you haven't already.

I am preparing the wiki updates. They will be ready soon. I'll let you know.

• Administration of REST API;
  • Administration of _sec-authentication Tables;
  • Multi-tenancy Administration;
  • Session Management Administration.

If there is something to be changed, added, or even removed, I will happily do it.

Ovidiu, I've made some minor edits to the main page.

For the existing SESSIONS and TENANT rest APIs, please create a summary for the REST paths which have changed (I think session unlock has changed at least); so customers already using this have an easier way to get what's changed.

Otherwise, please confirm this:

make sure the ACLs are honored by authorize

Eric/Greg: if testing is OK with 8976a, we need to release this before #9687, so the jetty changes are applied to 8976a (once is in trunk), too. Because we need to release to the customer both of these branches.

Constantin Asofiei wrote:

Ovidiu, I've made some minor edits to the main page.

Thank you.

For the existing SESSIONS and TENANT rest APIs, please create a summary for the REST paths which have changed (I think session unlock has changed at least); so customers already using this have an easier way to get what's changed.

I will do that today.

Otherwise, please confirm this:
make sure the ACLs are honored by authorize

Yes. They are. In the absence of the properly configured ACL, the response is 401 Unauthorized.
I also tested this branch against hotel_gui, customer's dockered harness smoke-test and a large application unit-tests, beside my local testcases. No regressions observed.

Constantin Asofiei wrote:

Eric/Greg: if testing is OK with 8976a, we need to release this before #9687, so the jetty changes are applied to 8976a (once is in trunk), too. Because we need to release to the customer both of these branches.

Agreed, but note that 9687a will need to rebase fairly frequently in general, so even if we miss something now we will pick it up soon.

Changes in configuration and runtime of REST APIs¶

        <node class="container" name="session">
          <node class="boolean" name="enabled">
            <node-attribute name="value" value="TRUE"/>
          </node>
          <node class="integer" name="timeout">
            <node-attribute name="value" value="600"/>
          </node>
        </node>

        <node class="container" name="admin-rest">
          <node class="boolean" name="enabled">
            <node-attribute name="value" value="TRUE"/>
          </node>
          <node class="integer" name="timeout">
            <node-attribute name="value" value="600"/>
          </node>
        </node>

         <node class="container" name="003100">
            <node class="resource" name="resource-instance">
              <node-attribute name="reference" value="SESSION:GET:/admin/sessions"/>
              <node-attribute name="reftype" value="FALSE"/>
            </node>
            <node class="webServiceRights" name="rights">
              <node-attribute name="allow" value="true"/>
            </node>
            <node class="strings" name="subjects">
              <node-attribute name="values" value="restAdmin"/>
            </node>
          </node>

         <node class="container" name="003100">
            <node class="resource" name="resource-instance">
              <node-attribute name="reference" value="ADMIN:GET:/admin/sessions"/>
              <node-attribute name="reftype" value="FALSE"/>
            </node>
            <node class="webServiceRights" name="rights">
              <node-attribute name="allow" value="true"/>
            </node>
            <node class="strings" name="subjects">
              <node-attribute name="values" value="restAdmin"/>
            </node>
          </node>

Changes in Session REST API¶

With r15837, the Session API has changed a bit in order to better adhere to REST recommendations.

1. GET /admin/sessions/login and GET /admin/sessions/logout.
   ❌ None is valid any more. The authentication is now handled in a common place. See above.
2. GET /admin/sessions/
   ✔️ If the body is empty, all sessions will be reported, as before.
   ✅ If the body contains a list of session identifiers, only the details of those specific sessions will be returned, ignoring the others. If a session has expired, it will be simply ignored. Example of payload:

   { "sessions": [ 1, 9, 2, 4, 100] }

   In this case only details on sessions 1, 9, 2, 4, and 100 will be provided, if they are still available. Any other session is ignored.
3. GET /admin/sessions/<session-id>
   ✅ New API. Will attempt to return information on specific session, identified by <session-id>. For example: GET /admin/sessions/16 will return

   { "sessions": [ {
       "logonTime": "2025-04-02T21:12:12.127+03:00",
       "accountDescription": "bogus",
       "description": "Bogus User",
       "osUser": "om",
       "sessionDescription": null,
       "sessionTokenID": "16",
       "sessionUUID": "101892fc-c5c2-440b-a6ce-d4d5d6640c98",
       "relatedSessionUUID": null,
       "programTrace": [
           "P8.2(line:73)",
           "P8.0(line:67)",
           "P8.execute(line:50)" 
       ],
       "clientWebPort": "0",
       "clientPID": "2074196",
       "browserSocket": null,
       "clientSocket": "/127.0.0.1:60958",
       "serverSocket": "/127.0.0.1:6333",
       "secure": true,
       "cacheGeneration": "1" 
   } ] }

   if such a session exists. The request body is ignored in this case.
4. GET /admin/sessions/lock
   ❗The semantic has changed. The old semantic was to enable locking, which prevented creation of new sessions. This was an incorrect usage of GET HTTP method. It was fixed so that it will return the current lock status. Response example when locking is not on:

   { "lock": false }

5. GET /admin/sessions/unlock
   ❌ None is valid any more. As with the above, this was breaking the REST recommendations and was removed.
6. PUT /admin/sessions/lock
   ✅ The old behaviour of previous two requests was extracted to this new one. It takes the argument in the body and sets the locking flag as instructed. To lock FWD from accepting new sessions use the following payload:

   { "lock": true }

   The FWD server will respond with:

   { "lock": true }

   to confirm the operation is successful. To enable session creation, use

   { "lock": false }

7. DELETE /admin/sessions/all
   ❌ None is valid any more. It was replaced with # DELETE /admin/sessions
8. DELETE /admin/sessions
   ✔️ If the body is NOT empty, the specified sessions will be dropped, as before. For example, a request with the payload:

   { "sessions": [ 1, 5, 3, 14 ] }

   will attempt to drop sessions 1, 5, 3, and 14, if they exist. The response will contain the list of sessions actually dropped.
   ✅ If the body IS empty, all active sessions will be dropped. They will be listed in the response, like:

   { "terminated": [ 3 ] }

9. DELETE /admin/sessions/<session-id>
   ✅ New API. Will attempt to drop a specific session, identified by <session-id>. For example: DELETE /admin/sessions/7 will drop session ##7 and return

   { "terminated": [ 7 ] }

   if such a session existed. The request body is ignored in this case.

Do you have Postman examples for /admin/security/domains?

When trying to list the domains I am getting this response

{
"errors": [
"Error initializing persistence services for database 'landlord'."
]
}

I am using this url since my shared db is named 'shared'

https://localhost:7443/admin/security/domains/shared

I think this is strange.
The (GET - I assume) for https://localhost:7443/admin/security/domains/shared will try to access/query the physical default database of the logical shared database.
It is not related to landlord database. landlord stores information for accessing the physical private tenant databases.

It is working now, I just needed to change it to the logical db name, not the physical. Thanks!

When trying to add a domain I am getting this

{
"success": false,
"errors": [
"com.goldencode.p2j.NumberedException: Creating records in a Virtual System Table is not allowed"
]
}

Is it not just adding a record to meta_sec_authentication_domain?

This is the payload

{
"_Domain-name": "SXE_TST9",
"_Domain-type": "_oeusertable",
"_Domain-description": "SXE_TST9",
"_Domain-access-code": "193c29282005666665677a",
"_Auditing-context": "",
"_Domain-runtime-options": "",
"_Domain-enabled": true,
"_Domain-custom-detail": "SXE_TST9",
"_Tenant-Name": "SXE_TST9",
"_Domain-Id": 5,
"_Domain-category": 0
}

Brett,
Your request is probably correct. Both _sec-auth tables were stuck as R/O. It an issue caused by a failed merge operation between two branches.
We discovered it in time and now it was fixed in r15850.

Constantin,
Is there a branch for the changes you proposed for fixing the context issues reported in #9687 which were not addressed by r15850?
If there is none, I can open a new branch on this task because I tested them and I identified some things which need adjustments.

Please create a branch and put the changes there. Thanks.

Ovidiu, do you have a branch for this?

Yes, 7080a. The changes were committed as r15860.

• this change from the patch in #9687 was omitted; the authorization ACLs work with the full path, right? i.e. ADMIN:GET:/admin/security/domains/. so passing just the target, without the full contextPath, is wrong. This change exists in AdminRestHandler, as authHandler.handle(getContextPath() + target, baseRequest, request, response);
  

  -      String token = authenticationHandler.authorize(target, request, response);
  +      String cpath = getContextPath();
  +      String token = authenticationHandler.authorize(cpath + target, request, response);
  
  

Otherwise, please confirm or otherwise test (with rest admin enabled) the FWD Admin Console and the Admin REST APIs, both should work.

The change was not omitted. I had to roll it back because the cpath will be added inside WebServiceAuth.authorize():274. The code is:

String contextPath = request.getContextPath();
[...]
String instance = [...] +
   Paths.get(contextPath, target) // path

With this code (no cpath added to authenticationHandler.authorize) Admin REST APIs works correctly for all admin modules (sessions/tenants/security).

However, I was not able to get it running in FWD Admin Console. The cause is

<script type="text/javascript" language="javascript" src="adminapp/adminapp.nocache.js"></script>

Ovidiu, did you do ./gradlew all ? The GWT part for FWD Admin Console does not get build for normal 'jar'.

I've tested 7080a again with Admin-Rest (Sessions), FWD Admin Console and soap/rest requests, and they all work.

What additional testing is needed for 7080a? The admin console is broken in trunk until this is merged, right?

• Admin Console
• REST ADMIN
• SOAP/REST with and without login path

All work. I think it can be merged.

That's good news.

In parallel I did test it with both smoke-test/harness and GUI navigation in the multi-tenant environment and it was also successful.
To be super safe, I could do the unit-tests of the customer application Eugene is maintaining. It will take me 1/2h.

Ovidiu Maxiniuc wrote:

That's good news.

In parallel I did test it with both smoke-test/harness and GUI navigation in the multi-tenant environment and it was also successful.
To be super safe, I could do the unit-tests of the customer application Eugene is maintaining. It will take me 1/2h.

I did not test the /web or /chui cases - can you test these, with admin rest enabled?

I do not know which project has /web to test with.

I did a /chui test with hotel. It did not end well. I got a intermediary screen but the connection was disrupted because of the following issue on client side:

java.lang.RuntimeException: Attempt to instantiate class class com.goldencode.p2j.ui.client.Image which has no proper mapping
    at com.goldencode.p2j.ui.client.driver.WidgetFactoryAdapter.create(WidgetFactoryAdapter.java:150)
    at com.goldencode.p2j.ui.client.UiUtils.reconstructComponent(UiUtils.java:1330)
    at com.goldencode.p2j.ui.client.WidgetRegistry.reconstructWidget(WidgetRegistry.java:326)
    at com.goldencode.p2j.ui.client.WidgetRegistry.pushDefinition(WidgetRegistry.java:766)
    [...]

java.lang.NullPointerException: Cannot invoke "com.goldencode.p2j.security.WebServiceResource.isAllowed(String)" because "this.webServiceResource" is null
    at com.goldencode.p2j.main.WebServiceAuth.lambda$authorize$0(WebServiceAuth.java:278)
    at com.goldencode.p2j.security.LegacyWebSecurityManager.lambda$checkAuthorization$1(LegacyWebSecurityManager.java:435)
    at com.goldencode.p2j.security.LegacyWebSecurityManager$WebServiceLogin.run(LegacyWebSecurityManager.java:556)
    at com.goldencode.p2j.security.ContextAwareThread.run(ContextAwareThread.java:136)
    at com.goldencode.p2j.security.AssociatedThread.run(AssociatedThread.java:80)

Related to the unit-tests I intentioned to run: I am momentarily unable to start the server correctly. The appservers refuse to start because The spawn process failed with 149 error.. I will investigate this.