As an end user I should be able to include my own endpoints.

[cyrano007]

As an end user I should be able to include my own endpoints.

As end user (no admin) I want to be able to add my own endpoints, so I can add my K8s test cluster or CF endpoints without asking the admin and they are not visible to other users

[richard-cox]

This shouldn't be too bad too implement. Though need to consider

• feature should be off by default and enabled by admin at deploy time
• add endpoint button should be visible only if is admin or the flag is enabled
• need to consider features in endpoints that are normally disabled via admin not adding endpoint (more specifically kube/helm features in non-kube environments, artifact hub, etc)

[nwmac]

@richard-cox Its a little more complicated I think - at the moment endpoints are effectively shared - they exist for all users across the system - and are added by admins.

I think we'd need to the notion of system endpoints as well as user endpoints - need to ensure we handle the case where an endpoint has been added by a user and then the admin adds it at the system level - we'd have the same endpoint in twice - the user's endpoints should probably always take precendence.

[cyrano007]

Hi, what does the Community Label mean for my request ?

[richard-cox]

Hi @cyrano007 , we track all our issues in github and tag those that come from outside the team with Community

[thquad]

Hello,

i would like to be assigned to this issue. What is the usual way of communication between developers (e.g. Slack)? As is stated in the docs, i would like to get in touch with the developers first to discuss my approach.

[richard-cox]

From slack

https://cloudfoundry.slack.com/archives/C80EP4Y57/p1610704654001800

Now i’m at a point where i’m implementing the logic for differentiating between user-endpoints and admin-endpoints. Since n users can create m endpoints, i thought about creating a junction table between endpoints and the respective creators of endpoints. But i think the cleanest approach would be to add a new column in the cnsi table, for the user-id who created the endpoint. If the same endpoint would be created twice, two records would be added to the table with a different user. This would prevent that configurations of one endpoint between users would affect all users.
Things to consider:

    All users should see all admin-endpoints.
    Endpoint-admins see their own endpoints too.
    Endpoint-admins should not be able to edit admin-endpoints. (same rights as current users have)
    Admins should see all endpoints and be able to edit them.
    For all endpoints which already exist before the code changes, records without a saved creator are considered to be created by an anonymous admin for backwards compatibility.

...

I think the approach generally looks ok. Below are some additional points to consider

• Frontend. At the moment the frontend handles endpoint state quite simply as admin (can register) and user (cannot register). This introduces a third state of user (can register). This will need to be taken into account when..
  • Behaviour when there are no endpoints. At the moment this is...
    • user logs in ... see's a custom screen and they can't do anything or navigate anywhere (it's a dead-end)
    • admin logs in ... redirected to endpoints page and custom text shown on page
  • Showing the add endpoint button '+'
  • Showing the edit action in the endpoint drop down
  • Showing the unregister action in the endpoint drop down
  • When registering an endpoint we show the option to 'share' it with other users. user specific endpoints should not be shared. this should be hidden if user is not an admin
  • Need to display some kind of indication that an endpoint is personal or global in the endpoints list.
• Need to consider how admins manage user specific endpoints. How are these endpoints added to the endpoints table... should they be kept separate? If included in the endpoints table, and any request of the admin to list endpoints, how are they stopped from using that endpoint themselves?
• Config
  • This should be disabled by default via env var. We handle these via config.properties/config.examples/etc and ./jetstream/repository/interfaces/structs.go PortalConfig
• DB
  • Heads up, any changes to the db need to be done via ./src/jetstream/datastore
  • when adding an endpoint the url must be unique (the cnsi guid is a kind of hash of the url). So would need to handle situations where the admin adds an endpoint with the same url that a user has
• Docs
  • Some initial docs on how this is configured and what it does should be added to somewhere in the /website docs

[thquad]

@richard-cox I'm almost done implementing all features before i switch over to adding/adjusting the tests, but i have a few questions which would be great if i could solve them first:

1. Since the ID of the users who created the endpoint are now saved in the CNSI table, i would like to display the firstname & lastname in the front-end with the endpoints. Is there a way to get this information from the not currently logged in user (session)?
2. Currently the ID of the user who created the endpoint can be read from the front-end. Should the ID be hidden from the front-end?
3. The database schema are edited with files in datastore/. I figured out that the first 8 numbers are the date with yyyymmdd (presumably when i created the file), but i don't know what the last 6 mean. Are they the specific time, as in hhmmss?

[richard-cox]

1. We don't have any way via the api to retrieve Stratos user information other than that of the person signed in. You will need to add an additional /user endpoint to the stableAdminAPIGroup in /srcjetstream/main.go. I suggest at this stage supporting only /user/:id which I think just needs plumbing in to p.StratosAuthService.GetUser

2. This opens up a wider problem, I don't think users should be able to see endpoints registered by other users. Which means they only see default system endpoints (create by admin) and their own. Admins are fine, they can see all.

3. The last 6 digits are HHMMSS. Like the YYYYMMDD digits before these are just used for ordering of updates.

[thquad]

Thank you for the response!

I was thinking that displaying the username instead of the first-/lastname could be a possible security issue, but since users can't see other user endpoints and only admins would see every created user-endpoint, this shouldn't be a problem either way. For the admins, i will replace the username with a generic "Admin", so that users won't see them.

I have removed the creator-id within the endpoint from the front-end, the id itself should not be useful in any case. Only a dedicated user-object similar to ConnectedUser will be sent with relevant information. The id will only be used internally / from the back-end.

EDIT: I have some additional questions regarding specific permissions and interactions between admins and user. The current plan / implementation is:

1. Users can't unregister or edit admin-endpoints.
2. Admins can unregister and edit user-endpoints.
3. Users and admins can connect to every visible endpoint. (Users only for their own and admin-endpoints, admins to every admin-endpoint and user-endpoint.) I gathered from your post 2 weeks ago, that admins shouldn't be able to connect to user-endpoints, but i think admins should have full permissions when it comes to endpoints.
4. Users can't share endpoints.
5. If an admin creates an endpoint, which already exists as user-endpoint (same APIEndpoint), the user-endpoints will be automatically unregistered. That way the admins are not required to manually unregister all duplicate user-endpoints first. Admins can't create the endpoint, if another admin-endpoint already exists.

Is this approach okay or are there objections for any of these points?

[richard-cox]

3. If this makes it simpler, ok. If the admin see's all there would be nothing stopping them adding the same endpoint themselves. Just had a thought on this, are you allowing admin's to register endpoints as a user?

4. This is ok, but I think it should not be automatic and executed by an additional admin option when registering an endpoint. This setting would cover both the frontend validation (disabling the unique endpoint api url validation) and the unregistering of clashing endpoints in the backend.

[thquad]

3. I'm currently thinking about two options: a) As you said, admins could just create their own endpoint to connect, which would delete the user-endpoint b) What if an admin wants to connect to a user-endpoints to configure something for the user quickly, but doesn't want to create their own endpoint for all other users to see? In this scenario the admin could choose to create their own or connect to the user-endpoint. To answer your question, i'm not allowing admins to register endpoints as a user. The current logic saves the id of the creator for each endpoint in the db. No additional configurations are saved.

4. I think you meant 5? That's a good idea, i will add another checkbox so that admins can enable overwrites.

[thquad]

Upon further consideration, I think it's best not to allow admins to connect to user-endpoints like initially suggested. The reason for that is to disallow admins or users to connect to the same URL twice and avoid any resulting problems.

[thquad]

Something came to mind while looking through the docs again and reading your answers.

In my understanding, the Backend Plugins are for adding endpoints, middleware and routes to Stratos. I implemented all my changes not as a plugin, since i'm not doing any of those (with exception of changing the middleware for the stableadmingroup) and the functionality of admin-endpoints and user-endpoints should work with any type of endpoints.

My current implementation in the backend, when the flag in config.properties is set:

• Added a creator column to the cnsi table, which stores the id of the user who created the endpoint.
• The cnsi guid will now be generated with the url + creator-id instead of just the url, since there can be multiple endpoints with the same url but different user.
• When endpoints are retrieved or edited, then the route middleware, info.go, cnsi.go, etc. will compare the scopes (admin, endpointadmin) of the current session user and the user who created the endpoint and act accordingly.
• All endpoints are saved in the endpoints table.

In my first post, i asked if implementing this issue with an added creator_id in the cnsi table would be okay, but i didn't specify how i would program this in the backend. Is my current implementation "correct", or should any of these be done differently in general (like with a plugin)?

[richard-cox]

Added a creator column to the cnsi table, which stores the id of the user who created the endpoint.

I think the db changes should not be gated on the config value. This would mean uses could then enable the feature later on if required. This does raise the question of what should happen if the feature is disabled on an existing system. If i understood correctly, the admin would just need to remove the endpointadmin scope from all users, delete their endpoints and then toggle the config setting. Might need some docs on that.

The cnsi guid will now be generated with the url + creator-id instead of just the url, since there can be multiple endpoints with the same url but different user

The cnsi guid should not include the creator's id. The predermined nature of url --> endpoint guid was added intentionally for a company. Isn't this information available via the cnsi record?

When endpoints are retrieved or edited, then the route middleware, info.go, cnsi.go, etc. will compare the scopes (admin, endpointadmin) of the current session user and the user who created the endpoint and act accordingly.

All endpoints are saved in the endpoints table.

Sounds good

In terms of plugins, as this is quite core adding outside of the plugin mechanism is fine. They're mainly used to add types of endpoints rather than common facets of one.

[thquad]

Thank you for the answers!

I think the db changes should not be gated on the config value.

I will change that so that the id of the user will always be saved.

This does raise the question of what should happen if the feature is disabled on an existing system. If i understood correctly, the admin would just need to remove the endpointadmin scope from all users, delete their endpoints and then toggle the config setting. Might need some docs on that.

Currently, if i disable the feature on an existing system, every endpoint will be visible for everybody. I should probably change that behaviour so that users won't see other user endpoints and only their own, even when the feature is disabled.

The cnsi guid should not include the creator's id. The predermined nature of url --> endpoint guid was added intentionally for a company. Isn't this information available via the cnsi record?

I couldn't find information on why the url is used to generate the cnsi guid. I will change it back, but that will prevent me from adding an endpoint with the same url with a different user to the table. I'm thinking of introducing a composite key in the table by setting the cnsi_guid and creator-id as primary keys in the db. Would this be fine instead?

---

I have also noticed today, that there is no user-invite option when creating a CF endpoint. The option to invite other users appears when i connect to a CF endpoint as a CF admin, as it's described in Configuring Invite User Support. A normal user, who connects as an admin to the CF endpoint is able to configure user-invites. Since this seems to be dependent on the CF roles instead of the Stratos roles, should i still disable this feature for stratos.endpointadmins?

[richard-cox]

I think the db changes should not be gated on the config value.

Sorry, i meant the creation of the column in the db should not be gated. Think tracking the id of endpoint creator is fine, but an empty entry should denote it was create by an admin. This should cover upgrade cases.

Currently, if i disable the feature on an existing system, every endpoint will be visible for everybody. I should probably change that behaviour so that users won't see other user endpoints and only their own, even when the feature is disabled.

I think with the feature disabled they should only see endpoints registered by an admin and not any they owned when the feature was disabled.

I'm thinking of introducing a composite key in the table by setting the cnsi_guid and creator-id as primary keys in the db. Would this be fine instead

This may work, but you would need to ensure changing PKs is supported by MYSQL and Postgres. We use sqlite in some cases, but I think we don't need to support upgrade for that one. @nwmac is this correct?

A normal user, who connects as an admin to the CF endpoint is able to configure user-invites. Since this seems to be dependent on the CF roles instead of the Stratos roles, should i still disable this feature for stratos.endpointadmins

That sounds like it's working as intended. The invite feature should be specific to the CF and the credentials used to connect to it. A Stratos admin might not be an admin of a CF and vice versa. In some cases the same uaa scope denotes an admin of both, but that isn't always the case.

[nwmac]

@richard-cox Should be okay. I think you can add multiple constraints.

[thquad]

Thank you! I will change it to the behaviors you described.

[thquad]

@richard-cox The cnsi guid should not include the creator's id. The predermined nature of url --> endpoint guid was added intentionally for a company. Isn't this information available via the cnsi record?

Could you explain why the guid is generated out of the url and how it is used? I'm wondering if the way it's generated could be changed.

I currently face several problems when thinking about altering the cnsi table (or creating a new table) to use the guid and > creator as primary keys:

• sqlite doesn't support altering primary keys. Whole table would be needed to be renamed, another table with same columns and configurations created and all entries moved to the new table (with the same name as before). Could possibly break other areas that would need fixing too.
• In several stores the endpoint is queried with the guid. Since the guid can be duplicates with user-endpoints, every instance (front-end, back-end, tests) where the guid is used to fetch an endpoint would need to be modified to also include the creator id.

I currently don't see a way around this. If i could change how the guid is generated, that would result in changing 4 lines instead of adjusting the whole source code. If nothing can be changed in how the guid is generated, then i will think some more about this and try to figure out a possible different solution. (maybe even a warning in the docs that the guid will be generated differently when activating this feature?)

EDIT: Upon further considerations, I could probably move endpoints to another table similar to how userfavourites are stored and query over that. Originally i was against the idea because i didn't want users editing the same endpoints, but i could store the individual configurations in the second table and just refer to the original endpoint with the guid. That would prevent adding the creator id to every query, since admins would fetch everything anyways and users could be identified with the sessiondata in the backend. Other parts of Stratos can still uniquely identify endpoints with the guid.

EDIT2: Ignore this quote-block above, i might have jumped the gun with the questions. Using two primary keys should work, adding another table or not, the execution stays more or less the same. I will try it out and come back later, once i spent a little bit more time on it.

EDIT3: I will sum up my thoughts from above in a more concise manner:

Could you explain why the guid is generated out of the url and how it is used? I'm wondering if the way it's generated could be changed.

If the guid can't be changed, then i'm running up against the same problem with every solution i'm thinking of. Since the guid will have duplicate entries, i can't use it to identify endpoints anymore.

Possible solutions:

1. Altering the table to have 2 primary keys. This would result in: a. Since i can't change primary keys in SQLite, i would need to rename the table, create an identical empty table, transfer everything to the new table and drop the old one. I'm not sure what effect that will have on existing systems. b. I would need to edit each instance, in which an endpoint is fetched with the guid to also include the creator id. That means editing all plugins, front-end, tests, routes, etc. This approach seems a little bit too disruptive in my opinion. That also means including the id of the creator in the json data. (i'm not sure how security would fit into that, if it would need to be encrypted?)
2. Adding another table for user-endpoints a. Basically the same issue, that i would need to change everything to also include the id of the creator. b. I could hide the guid behind a user-endpoints-guid. If a request can't find an endpoint in the cnsi-table, then it will look up the user-endpoints-table, which in turn will know the "real" guid and fetch the endpoint. I'm unsure if it will conflict with the original purpose of the guid and if this would break something? c. Disallow admins to edit or unregister (individual) user-endpoints (also see user endpoints). This would solve the issue of also sending the creator id, since endpoints could be identified purely by the guid and who is logged in at the moment. Admins can still create a new endpoint that would delete all user endpoints with the same url. (endpointadmins would create copies of one endpoint in the cnsi table on their user-endpoints table. Functions like ConnectedUser on an endpoint in the tokens table would point to the unique endpoint in the cnsi table. I'm not sure if this will work or if there are any other parts of Stratos that would break with that approach)
3. Clarify in the docs, that the guid will be generated differently and let the person setting up Stratos decide if they are allowed to do that, when activating the flag.

What are your thoughts on the matter?

EDIT4: Added 2.c. to the solutions.

[richard-cox]

I've managed to find the original explanation why the cnsi guid was made deterministic - https://github.com/cloudfoundry/stratos/pull/3031#issuecomment-423323347. With that in mind as long as your changes don't break their requirement you're free to update the cnsi guid. In their case I believe the link would only ever be required for administrator added endpoints so as long as those hash out to the same result their approach will still work. This is a similar use case to upgrade, where previously added endpoints won't have any user info in the cnsi guid. Also I would agree it should be made clear in the docs that enabling the feature would change the deterministic behaviour. So I guess that's option 2.c with a caveat

[thquad]

This is very helpful to know. So if i understand correctly, i can do the following:

• Admin endpoints will still generate the same guid as before
• User endpoints will generate their guid through a combination of url + creator id.
• In the docs it will be made clear, that enabling this feature will change the way guid will be generated for user endpoints.

[thquad]

I think with the feature disabled they should only see endpoints registered by an admin and not any they owned when the feature was disabled.

@richard-cox if the feature is disabled, should the admins still see all user endpoints?

[richard-cox]

I think going on the principal that admins are all seeing they should, though this may cause issues in the frontend when deciding to show extra endpoint information given the enabled/disabled flag. Maybe there could be an additional flag to allow admins to see user endpoints, or the existing flag could be an enum (enabled, partiallyDisabled, disabled).

[thquad]

Then i will implement it with an enum.

Another question: Currently i look up the creator to see if they are an admin or endpointadmin, but what if a user can't be found because they don't exist in the db? Is it even possible that tokens are not stored in the db, because there seems to be no way to delete uaa tokens currently? @richard-cox With the current implementation, when the flag is enabled, admins and endpointadmins id will be saved with the endpoint entry. I'm thinking of never saving the admin id, so that endpoints can always be distinguished between admins and users by the fact that an id is saved. The problem with that, what if a user gets promoted to an admin, i would still need to look up the user.

[richard-cox]

Users will almost always come from an external source and not stored in the db. The only exception is a kind of 'trial' admin that's stored in the local_users table. I don't think we've previously hit the case where we could request a user that has since been deleted, as the user was always the one that was logged in. I think we'd have to handle this with some kind of 'Unknown' interpretation of a missing user. In terms of saving the user id I'm not sure of the problem. Could the lookup not be avoided if the user id is missing, which would mean it was created by an admin? We lose some traceability here, but it's not something we have at the moment anyway. The admin can always create their own personal endpoints with would contain their own user id. Those would be treated as any other endpointadmin created endpoint.

[thquad]

Thank you, I wasn't explaining myself well enough but your answers already clarified some questions for me.

Issues:

1. User can't be found
2. User creates endpoint -> User gets the admin role afterward -> Endpoint will have the wrong guid (url + user-id) and be visible for every user, since it's an admin endpoint now.
3. Admin creates endpoint -> Admin gets demoted to user -> Endpoint will be handled as user-endpoint and therefore have the wrong guid (url instead of url + user-id) and removed from view for all other users

Solutions:

1. I will treat user as unknown non-admin (endpointadmin)
2. Would need to check if user roles changed after every login. (Probably when the auth token is fetched to compare old and new tokens (roles changed) in the token table, but i'm assuming those can be compared) -> Update endpoints accordingly: a. Remove previously registered endpoints by that user. b. Update guid and let users see all endpoints from new admin. -> All user endpoints with same url would need to be removed, since i don't allow a user endpoint to have the same url as an admin endpoint. c. Let them exist as user-endpoints only visible for the new admin to see.
3. Don't save the id when an admin creates an endpoint. Endpoint will be detached from the admin-account and not change when the account changes (since endpoints with no saved id are considered admin-endpoints).

Problems: 2.c. With my current implementation, i don't support user-endpoints for admins.

For 2, i think 2.a. seems to be the most sensible approach, thinking from a UX perspective.

To further explain why 2.c. is tricky to implement and to state again how i programmed this whole feature so far:

• A user-id is added to the cnsi-table as "creator".
• During lookup of endpoints (backend), the endpoints will be filtered by who is currently logged in and who created the endpoint.
• User see endpoints where the user-id match "creator" OR "creator" has the admin role (see their own and all admin endpoints). That's why all previously created user-endpoints would be seen as admin-endpoints when the user gets the admin role after the fact.

Endpoints created by an admin are therefore always non-personal, unique (url) and can be seen by all users.

Are there any objections or remarks for stuff i might not be considering? If not then i will try to implement solutions 1, 2.a. and 3.

EDIT: Sorry, for some reason, i didn't consider in 2c that i could remove the check-for-admin when an id is available and just assume that it's a user-endpoint, like you mentioned in your comment. That would make more sense, because the system-endpoints are visible for everyone and shouldn't be bound to a specific user. That way we also leave the option for admins to create their own user endpoints open. I think i will go for 2c instead of 2a now.