GALAglobal / TAPICC-API-implementation

TAPICC API implementation using node.js framework sails.js
Other
6 stars 1 forks source link

userId is a number but it should be a string #4

Closed Alino closed 6 years ago

Alino commented 6 years ago

this is a feedback coming from Rodolfo. I would like to ask why should userId be a string and what kind of string?

rmraya commented 6 years ago

This API should allow integration with different systems. If in an existing system the user ID is a number, it can be converted easily to a string. On the contrary, if the user ID is a string, like "admin", it cannot be converted to a number and integration between systems would not be straight.

Alino commented 6 years ago

Thanks for explaining that to me. Could you please tell me more in detail how would the user data be integrated with a different system? An example would be great.

rmraya commented 6 years ago

SSO (single sign-on) servers are common these days. For example, you can login to https://gitter.im/ using your GitHub account to view this post. These would be two separate systems with one shared user id.

There are many systems that allow you to login using a Google account. The user ID would be an email address, not a number.

Alino commented 6 years ago

as far as I know, OAuth systems like "Login with Facebook" or "Login with Github" can work independently of your existing authentication mechanism and user data. We could create a separate user table for each 3rd party OAuth (which is a good/best practice). This separate user data could be connected together trough a common unique parameter, in most cases it's email. But the user ID can still be auto-incremented integer.

rmraya commented 6 years ago

The extra mapping from integer to string should not be necessary.

Alino commented 6 years ago

if we move 3rd party user ids into our user table, and they will be mixed with our users that were created through TAPICC, there could be a potential conflict of ids. How would our userId look like if it were a string? Some random generated hash?

rmraya commented 6 years ago

The implementation must check if the ID exists before adding a user through the API. In case it exists, an error or warning should be returned.

The ID value should be provided by the tool requesting the creation of a user. You don't need to worry about how it looks like.

rmraya commented 6 years ago

If user IDs are generated by the tool consuming the API, it would be easier for the tool to manage tasks related to users because it would not have to map internal lDs to IDs coming from the API.

Alino commented 6 years ago

good idea, and for our internal user ids, we could use a prefix + numeric id. Something like tapicc_1 If we do it like this, then it's very unlikely to have an ID conflict with users created in TAPICC. However there still could be a conflict if the TAPICC server would be integrated with multiple different systems at once.

rmraya commented 6 years ago

There is no need for an internal user ID. The one provided by the tool that creates the user is enough.

Alino commented 6 years ago

So the user creation would be dependent on an external system, with no way for the admin to create a user independently?

rmraya commented 6 years ago

Who says the administrator can't create a user?

Alino commented 6 years ago

Sorry I though you wanted to delegate the user creation just for the external system. So how would the user creation work? What are all possible scenarios?

Here are few possible scenarios that came to my mind with the current context:

Scenario 1 - admin creates a user manually Given that the TAPICC admin knows the user ID in the external system of the user that he wants to create also in TAPICC And the user ID is 12345 When the admin makes a POST request to /register with this data { id: '12345', password: 'secretPass' } Then a user with id 12345 is created


In these next scenarios, a TAPICC plugin is required so that the user or admin can perform the user creation. This TAPICC plugin obviously doesn't exist yet. But is something that could be written for each external system.

Scenario 2 - user initialises registration request Given that the user of an external system wants to become a user in TAPICC And the user ID is 12345 And he is currently using the external system When the user clicks on a button "copy my profile to TAPICC" Then the external system performs a request to /register with this data { id: '12345', password: 'secretPass' } Then a user with id 12345 is created

Scenario 3 - admin creates a list of users Given that the admin is currently using the external system When the admin clicks on a button "copy users to TAPICC" Then he is given a list of users on the external system where he can select and deselect users who he wants to copy to TAPICC. Then all these users are created in TAPICC database And default password for these users is default123

rmraya commented 6 years ago

Only administrators should be able to create users, using whatever tool they like.

Note: a TAPPICC server should not store a password. That's dangerous. Only a hash of the password should be stored.

Alino commented 6 years ago

So do you agree with Scenario 1? The admin is required to specify the user id. It is never generated by TAPICC.

How should be a user ID conflict handled (after we show the warning), if there are two external systems connected to TAPICC? Or will it never be a case that more than one external system integrates with TAPICC?

PS: of course we will hash password.

rmraya commented 6 years ago

To avoid problems, the tool used to create a user should check if the ID already exists before adding the user. Collisions should not happen if the administration tool is well designed.

Alino commented 6 years ago

If there are 2 external systems, and both have users with numeric IDs which are auto-incremented. It's very likely there will be many collisions... This is a huge problem. How should this situation be solved? I understand that we will show the admin a warning message that there is a collision. But what then? What can the admin do about this situation if he still wants to create the user? Is he supposed to make up an id?

rmraya commented 6 years ago

That's a symptom of very bad design for the API client. A well-designed tool should handle the creation of IDs in general, not only for users. The ID of a group, a job, a project, a group and whatever, should be something unique and should be assigned by the creator of the entity, not by the backing server.

Alino commented 6 years ago

Sorry I don't understand. I thought the two external systems could be independent of each other and both have their own user data. If a single instance of TAPICC would integrate with more than one external systems, in a way that it would reuse the very same user ids from those external systems, there will be collisions. And we need to specify what should be done in such case other than showing the admin a warning message.

Could you please give me an example of an external system? Or even better, do you a have a list of such systems in your mind? I would like to take a look how do these external systems handle their user data, especially user ID

rmraya commented 6 years ago

With good planning, there are no collisions. If integration between systems is well done, users and all other entities can get a unique id.

I have actual systems that could support this API if it is well done. These systems are already deployed and interacting with tools designed by other vendors.

Creation of element IDs is something that doesn't belong to the API. It is something that client tools should handle.

Alino commented 6 years ago

From our conversation, my understanding is that the external system generates the user ID when the user is created there. Then the admin of the TAPICC api will create a new user in TAPICC, with the same user ID as the user ID in the external system.

But how can good planning help avoid collisions in this case, if the external system is not something created by us, and we need to adapt to it?

Am I missing something?

rmraya commented 6 years ago

An administrator of an external system creates the user locally AND in the TAPICC server simultaneously.

Every external system verifies ID availability against the server before creating any object.

As all systems verify ID availability before creating objects, there are no duplicated IDs anywhere.

Alino commented 6 years ago

The external system is still a complete blackbox for me. I don't even know what it is. And I have zero knowledge how it works and what is the purpose of it.

Since I lack all these information, maybe I am asking stupid questions now:

  1. are you sure there never could be a situation where the admin of the external system would want to create the TAPICC users later and not simultaneously?
  2. is it possible that a single TAPICC instance would use more than one such external system? (where the user ids are same)
rmraya commented 6 years ago

1) The external system may have users that are only local, users that are local and connected to a TAPICC server and users that are only authenticated via a TAPICC server. This is not relevant at all for the design of the API. It is a problem that should be managed in each client. 2) Yes.

FWIW, I've been writing this kind of system during the last ten years.

Alino commented 6 years ago

I think that most applications are generating their user IDs automatically. That's why I need to ask:

  1. Could there be an external system which doesn't let you create a user with a specific ID? But is rather generated automatically?
  2. How many different types of external systems which would be connectable to TAPICC could there be?
  3. Is Drupal CMS a valid example of an external system where TAPICC would reuse the same user ID?
Alino commented 6 years ago

I think there could be a problem if a single instance of TAPICC is connected to external system A. But some time later, you want to add another external system B to this same TAPICC instance. But that external system B, would have collisions with user ids in A. Could this be a case? If not, why not?


If this is going to be a problem then we could also do a different approach. We can have user ids generated automatically by TAPICC api. But the API endpoints related to user manipulation would also accept other unique user identifier such as email address.

rmraya commented 6 years ago

If you prefer to generate IDs in the TAPICC server, do so. If you prefer to use an integer as ID type, do it. I give up.

stiofanmacthomais commented 6 years ago

Why can't the consuming systems simply map user id's themselves as needed, and handle authentication/authorization/accounting? Why would this be a TAPICC concern? TAPICC would then simply provide the ability to contain user id's as "useful metadata" them but not really care about them.

If TAPICC extends into something like OpenId well, it's just my opinion, but that'd be out of scope for a translation API framework.

In reality, if I have a user ID { user : "stephen@domain.com" } and a job with this information is being processed by a given TMS, then either 1. the ID will match a corresponding TMS managed Id or 2. The consuming application will map "stephen@domain.com" to 607ed0a6-8c79-4dae-8cb3-4c007317db5d or whatever it needs to associate the request for internal processing needs.

stiofanmacthomais commented 6 years ago

In thinking about this some more, the notion of using email addresses or "verifiable information" about a user may not be a good approach. Perhaps both the REST caller and REST receiver should expect a user ID to be in GUID format (i.e., anonymous) and mapped as needed on either side of the endpoint.

Although previous correspondences on security (and encryption specifically) pointed to the fact that data manipulation via a man-in-the-middle attack might be prevented through the use of HTTPS, I wouldn't be satisfied with this alone.

Thoughts?

ysavourel commented 6 years ago

FYI: See also issue #24

Alino commented 6 years ago

Closing in favour of #24