Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use /security/login instead of /login? #44619

Closed
azasypkin opened this issue Sep 2, 2019 · 3 comments
Closed

Use /security/login instead of /login? #44619

azasypkin opened this issue Sep 2, 2019 · 3 comments
Labels
discuss Feature:New Platform Feature:Security/Authentication Platform Security - Authentication Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!

Comments

@azasypkin
Copy link
Member

azasypkin commented Sep 2, 2019

Do we see any issues with having /security/login instead of /login in the next major apart from the fact that it will be a very tedious work to update all the docs and tests that reference to /login?

Context

Currently NP plugins can only define routes that start with plugin id or with /security in our case.

And since we don't tend to maintain (?) BWC for API routes URLs exposed by plugins in minors, it shouldn't be a problem for majority of our API routes to migrate to this new "prefix". But there are some routes that we can't break in minors, e.g. routes that are used by external SAML/OIDC providers like /api/security/v1/saml. The initial idea was to keep old routes in LP and new routes in NP, but in practice it's quite cumbersome since routes in LP and NP may rely on the same logic (see #44513 for example) that we need to share between LP and NP somehow. To make migration easier platform/core may allow us to define routes that are relative to the root instead that we can deprecate in 7.x and completely drop in 8.0 (#44620). All is well with API routes here.

But the question here is it feasible/acceptable to use /security prefixed URLs for "well-known" views like /login?

cc @elastic/kibana-security @restrry

@azasypkin azasypkin added discuss Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more! Feature:Security/Authentication Platform Security - Authentication Feature:New Platform labels Sep 2, 2019
@legrego
Copy link
Member

legrego commented Sep 3, 2019

But the question here is it feasible/acceptable to use /security prefixed URLs for "well-known" views like /login?

I think it's acceptable, but I do like the look of /login better than /security/login, for whatever that's worth. (similarly for /logout vs /security/logout).

If we didn't have the SAML/OIDC endpoints to contend with, I'd ask if /login and /logout could be provided by core hooks since auth itself is a part of core. But, since it sounds like we'll need a way for plugins to define routes relative to the root, it might not be worth that effort if we can use the same routing logic between our two needs.

@azasypkin
Copy link
Member Author

but I do like the look of /login better than /security/login

Yeah, same here. Let's see, there is a chance in #44620 we agree to turn temporary fix with isRelativeToRoot route option into something permanent and officially supported by the core that'd let /login to stay in 8.x+ too.

@azasypkin
Copy link
Member Author

Based on the result of discussion in #44620, we'll still have a way to register "resources/views" with URLs that are relative to root, so we can continue using /login in 8.0+, e.g.:

httpResourceService.add({
  path: 'login',
  fromRoot: true,
  handler(){}
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Feature:New Platform Feature:Security/Authentication Platform Security - Authentication Team:Security Team focused on: Auth, Users, Roles, Spaces, Audit Logging, and more!
Projects
None yet
Development

No branches or pull requests

2 participants