Giter Site home page Giter Site logo

authenticator's People

Contributors

gcurbelo123 avatar jlooney avatar joestubbs avatar scblack321 avatar scleveland avatar wesleyboar avatar

Stargazers

 avatar

Watchers

 avatar  avatar  avatar  avatar  avatar

authenticator's Issues

Support application/x-www-form-urlencoded content type for POST /oauth2/tokens endpoint

The OAuth2 specification states that the application/x-www-form-urlencoded content-type should be used for the /tokens endpoint. Currently, authenticator breaks from the spec and only supports application/json content type, as this is the only supported content type in Tapis generally. However, this causes client oauth libs (e.g., Django's authlib) that adhere strictly to the spec to fail when trying to integrate with Tapis. Therefore, we should make an exception and allow application/x-www-form-urlencoded content for this one endpoint (only).

Make MFA expiry configurable

Currently, once a user has passed an MFA challenge, this fact is cached in the session and the user is not required to pass MFA again until the session expires or the user logs out, etc. We should extend the MFA config for a tenant to allow a tenant to set a maximum TTL for an MFA and require the user to re-MFA after the TTL has been exceeded.

Add support for multiple IdPs

Create a new oa2 extension type, "multi_idps", allowing tenants to configure multiple IdPs. There should be a new end-user facing page allowing users to select which IdP they wish to login with. This should support LDAP IdPs in addition to OIDC types.

Consider defining "SK" before use in README

Consider defining the word "SK" in the README before using it.

Background

In the README, the word SK is used without definition. Searching "sk abbreviation" in google gave me off-topic results*. I learned, by asking devs, that it means "security kernel". The first search result I found—that actually has "security and "sk" in it—was TAPIS docs*, so I think it might not be widespread enough to use without explanation.

* I understand my past Google searches may influence the results.

Support for Implicit grant type

The implicit grant type is a standard in OAuth2 and will be needed for "pure front-end" applications such as the Tais UI.

IDP Form Has Unexpected Markup

Overview

Radio buttons on dev oAuth IDP form are using incorrect markup.

Important
This issue proposes a quick fix to the markup.
Another issue offers a new design for the choices:

Steps to Reproduce

  1. Be in a state like that of a user who has not visited Tapis.io.
  2. Open https://dev.develop.tapis.io/v3/oauth2/idp.
  3. See a form with radio buttons.

Expected Result

Radio button labels are on the same line as the radio buttons.

Actual Result

Radio buttons are on a new line underneath the radio buttons.

Proposals

A. Use Markup Expected by s-form Class1

Snippet of Expected Markup Applied to Problem Form
  <!-- ... -->
  <div class="has-required">
    <label for="idp_id">Account Type <span aria-label="(required)">*</span></label>
    <menu>
      <li>
        <label>
          <input id="prod_kc" type="radio" required name="idp_id" value="prod_kc">TACC Keycloak
        </label>
      </li>
      <li>
        <label>
          <input id="dev_kc" type="radio" required name="idp_id" value="dev_kc">Dev keycloak
        </label>
      </li>
      <li>
        <label>
          <input id="dev_ldap" type="radio" required name="idp_id" value="dev_ldap">Test Accounts
        </label>
      </li>
      <li>
        <label>
          <input id="github" type="radio" required name="idp_id" value="github">GitHub Accounts
        </label>
      </li>
      <li>
        <label>
          <input id="globus" type="radio" required name="idp_id" value="globus"> Globus Auth
        </label>
      </li>
    </menu>
  </div>
  <!-- ... -->
What Makes This Class Special?

The s-form class is a "Scope" namespace (hence the "s-"). It has an inflexible expectation of the markup inside it.

The alternative to s-form class1 is c-form1, which requires a class on most elements in the form. It is tedious, but gives you some more freedom with the markup tags and usage.

Why Those Tags?
  1. It is common to use a <div> is used to wrap all elements required for a any one field. But, a <div> (or other non-semantic wrapper) is not necessary for every single <input>, because <input>s of type="radio" (and type="checkbox") function together as a combined input. Such inputs can be grouped within a single wrapper.
  2. The collection of <input type="radio"> buttons act as one option, so they—as a single group—merit a label (<label for="idp_id">…). The <label> element can be separate from the field it labels, if it uses the for attribute.
  3. The <input type="radio"> buttons are a menu (<menu>) of interactive elements.
  4. All <input type="radio"> buttons are required because "To improve code maintenance, it is recommended to either include the required attribute in every same-named radio button in the group, or else in none."
  5. Each input also needs a label to describe it (<label>). The <label> element can wrap inputs and text, in which case it does not need the for attribute. Wrapping the input and text this way and order lets those nodes naturally be on one line, and puts the text after the "button".
And… required?

The has-required class lets CSS know that the field within (e.g. <input>, <select>, <textarea>) has the required attribute, so it can style parent or sibling elements (of the required element) accordingly. Example, the <span aria-label="(required)">*</span> creates a stylized "(required)" flag on the label. You will not see that on s-form--login forms, because there is a style to specifically hide those. If you removed the s-form--login class1, then that "(required)" would show up, like in s-form class demo1.

I tested changing the markup on the live page.

Before After
before after

Footnotes

  1. These links do not work as of the posting of this issue. I will update the server soon so that the links work. 2 3 4 5

fix error message on client delete for clients that have generated an authorization code

Currently, clients that have generated at least one authorization code cannot be deleted due to an SQL constraint. I think we want this behavior (i.e., to not allow client delete after generating an authorization code) for auditability/tracability purposes, but we should have a better error message. We probably also need to allow users to deactivate the client (i.e., a "soft delete") to help with clean up.

Add AccessTokens and RefreshTokens table

We should create two SQL tables that together hold a record of all tokens generated by the service to allow us to audit any suspicious token and to report on various metrics associated with Tapis (e.g., total unique users who have generated a token).

We could combine into a single table, but that would increase the size of the table, close to doubling it, and the table size could be an issue long-term. It would also require the queries for various metrics (e.g., count all access tokens generated by a single user in a given tenant with a specific client) to be more complex because they would have to select only tokens of a specific type.

AccessTokens table fields:
token_jti (str)
subject (str)
username (str)
tenant (str)
client_id (str, FK)
grant_type (str)
with_refresh (T/F) -- whether a refresh token was generated
create_time (time)
revoked (T/F)
revocation_time (time)

RefreshTokens table fields:
same as AccessTokens table but without with_refresh

Add support for Globus authentication as a custom idp configuration

While #18 adds support for various authentication methods via KeyCloak, including Globus, we have determined that adding support to utilize Globus authentication directly will confer additional benefits, including removing dependence on a separate KeyCloak instance and the ability to derive additional metadata about the authentication by making requests to additional Globus endpoints (e.g., token introspection endpoints). For more details, see https://confluence.tacc.utexas.edu/pages/viewpage.action?spaceKey=CIC&title=Federated+Identity

Thus, this issue will add a new custom_idp_configuration of type "globus" that will support walking OAuth flows directly with the Globus Auth server. Some prototype code that shows the example requests and responses is available here: https://gitlab.tacc.utexas.edu/rcardone/oauthtest

Add support for OAuth2 Provider IdPs

Currently, authencticator supports binding to LDAP to check a username and password. It provides clients with OAuth2-compliant endpoints to authenticate users. The point of this extension is to support additional types of Identity Provider systems (IdPs); specifically, IdPs backed by OAuth2 provider servers.

The primary use cases for this feature include:

  • Supporting the custom IdP for the CII tenant, being developed as an OAuth2 provider server
  • Support for login with InCommon, for example, via the Custos project.
  • Support for login with other major OAuth providers, such as Github.

A full write-up on the background and design for this feature is included in this google doc

Tasks in this Issue:

  • Design and extend the custom configs jsonschema to support OAuth IdP extensions.
  • Implement an OAuth2ProviderExtension class with basic utilities for interacting with an OAuth2 provider server.
  • Implement GET /v3/oauth2/extensions/oa2/callback as the OAuth2ProviderExtCallback controller
  • Update WebappTokenAndRedirect in controllers.py to check for OAuth2 extension config.
  • Make any updates to create_clients_for_tenant() and associated call chain.
  • Update the TenantConfigResource to allow any tenant to be selected, including tenants with custom OA2 IdPs.
  • Configure a github tenant and test with callbacks for localhost, developm staging and production.

API request intermittently results in sqlalchemy error

Occasionally, calls to different endpoints return an uncaught exception error
sqlalchemy.orm.exc.DetachedInstanceError: Instance <TenantConfig at 0x7f5ab85f9f50> is not bound to a Session; attribute refresh operation cannot proceed (Background on this error at: https://sqlalche.me/e/14/bhk3

Background on the sqlalchemy error indicates the issue has to do with accessing objects in a Session after it has been closed.

Here is an example full trace:
File "/usr/local/lib/python3.7/site-packages/common/utils.py", line 82, in handle_error raise exc File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1949, in full_dispatch_request rv = self.dispatch_request() File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1935, in dispatch_request return self.view_functions[rule.endpoint](**req.view_args) File "/usr/local/lib/python3.7/site-packages/flask_restful/__init__.py", line 458, in wrapper resp = resource(*args, **kwargs) File "/usr/local/lib/python3.7/site-packages/flask/views.py", line 89, in view return self.dispatch_request(*args, **kwargs) File "/usr/local/lib/python3.7/site-packages/flask_restful/__init__.py", line 573, in dispatch_request resp = meth(*args, **kwargs) File "/home/tapis/service/controllers.py", line 130, in get if tenant_configs_cache.get_custom_oa2_extension_type(tenant_id=tenant_id): File "/home/tapis/service/models.py", line 187, in get_custom_oa2_extension_type config = self.get_config(tenant_id) File "/home/tapis/service/models.py", line 171, in get_config if t.tenant_id == tenant_id: File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py", line 481, in __get__ return self.impl.get(state, dict_) File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py", line 926, in get value = self._fire_loader_callables(state, key, passive) File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/attributes.py", line 957, in _fire_loader_callables return state._load_expired(state, passive) File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/state.py", line 710, in _load_expired self.manager.expired_attribute_loader(self, toload, passive) File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/loading.py", line 1395, in load_scalar_attributes "attribute refresh operation cannot proceed" % (state_str(state)) sqlalchemy.orm.exc.DetachedInstanceError: Instance <TenantConfig at 0x7f5ab85f9f50> is not bound to a Session; attribute refresh operation cannot proceed (Background on this error at: https://sqlalche.me/e/14/bhk3) [in /usr/local/lib/python3.7/site-packages/common/utils.py:87]

Add client update endpoint

Add support for updating a client via a PUT request. Only the owner can update client. We should also allow clients to be deactivated (i.e., "soft delete") for clean up -- we don't allow users to delete clients that have generated authorization codes for posterity.

Support configurable TACC MFA at the tenant level

A tenant should be able to configure TACC MFA to be required for one or more (or all) grant types. Note that for this issue, use of the TACC MFA requires use of the TACC IdP: incorporating TACC MFA into tenants using a different IdP (for example, through an account mapping) is beyond the scope of this issue.

Tapis Developer Admin Page

We need a simple web page where Tapis users/developers can administer aspects of Tapis. This includes:

  1. Create and modify OAuth clients using a web form.
  2. Generate access tokens including long-lived tokens.

Long-term, it should also link to/include tapis-ui or something similar to allow users to manage other Tapis objects. In particular, "administrative users" will need to do things like manage roles, update configuration of authenticator itself (e.g., for their tenant), update the signing keys, etc. It could also show usage statistics (e.g., unique users in their tenant that have generated an access token).

Design and implement support for custom tenant configs

Authenticator configs should be a separate resource attached to a tenant, to include things such as

  • max and default access_token_ttl
  • max and default refresh_token_ttl
  • customizations to login page
  • grant types allowed, including impersonation
  • MFA required
  • password grant without client? i.e., "default" tenant client.

The design needs to be flexible, for example a JSON blob with associated JSON schema.

Tasks in this issue:

  • Design configs object and json schema
  • Add TenantConfigs model and SQL table
  • Add GET, PUT /tenants/admin/config endpoints
  • Update openapi spec
  • update auth.py to check SK for tenants admin role on /admin endpoints
  • update controllers.py to honor custom configs (e.g., token_ttl, etc.)

Upgrade to tapipy 1.4.0

The 1.4.0 release of tapipy brings a lot of good upgrades including much faster loading of specs. It does upgrade the version of openapi-core as well though, so we just need to test that it does not break the RequestValidator() stuff.

response_type dropped in login template, causes issue in Implicit flow

Currently, the client_response_type is not being passed along during the login process, and this causes an issue for the implicit flow. The fix is to update the service/templates/login.html template to store the client_response_type in the form, as is being done in all of the other templates.

Key (tenant_id)=(*) is duplicated

The migrations Job fails. The pod goes to the Error state and gets started again until it runs out of retries. This is reproducible by doing "kubectl apply -f migrations.yml". The logs for the pods contain:
psycopg2.errors.UniqueViolation: could not create unique index "uq_tenantconfig_tenant_id"
DETAIL: Key (tenant_id)=(*) is duplicated.

Full logs:
2023-06-05 18:27:49,534 INFO: returning a logger set to level: INFO for module: tapisservice.tenants [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:27:54,087 INFO: returning a logger set to level: INFO for module: tapisservice.auth [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:27:54,087 INFO: returning a logger set to level: INFO for module: service [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
Got exception trying to unpickle and create spec for spec_path: "files"; exception: [Errno 2] No such file or directory: '/usr/local/lib/python3.7/site-packages/tapipy/specs/tapis-project-tapis-files-dev-api-src-main-resources-openapi.pickle'
Falling back to prod spec for "files" resource
2023-06-05 18:28:00,864 INFO: returning a logger set to level: INFO for module: tapisservice [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,046 INFO: returning a logger set to level: INFO for module: tapisservice.tapisflask.utils [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,204 INFO: returning a logger set to level: INFO for module: tapisservice.tapisflask.resources [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,342 INFO: returning a logger set to level: INFO for module: service.models [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,345 INFO: returning a logger set to level: INFO for module: service.models [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,475 INFO: returning a logger set to level: INFO for module: service.ldap [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,475 INFO: returning a logger set to level: INFO for module: service.auth [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,513 INFO: returning a logger set to level: INFO for module: service.oauth2ext [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,515 INFO: returning a logger set to level: INFO for module: service.mfa [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,515 INFO: returning a logger set to level: INFO for module: service.controllers [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,515 INFO: returning a logger set to level: INFO for module: service.api [in /usr/local/lib/python3.7/site-packages/tapisservice/logs.py:43]
2023-06-05 18:28:06,529 INFO: Authenticator ready [in /home/tapis/service/api.py:99]
/usr/local/lib/python3.7/site-packages/flask_sqlalchemy/init.py:835: FSADeprecationWarning: SQLALCHEMY_TRACK_MODIFICATIONS adds significant overhead and will be disabled by default in the future. Set it to True or False to suppress this warning.
'SQLALCHEMY_TRACK_MODIFICATIONS adds significant overhead and '
/usr/local/lib/python3.7/site-packages/flask_sqlalchemy/init.py:835: FSADeprecationWarning: SQLALCHEMY_TRACK_MODIFICATIONS adds significant overhead and will be disabled by default in the future. Set it to True or False to suppress this warning.
'SQLALCHEMY_TRACK_MODIFICATIONS adds significant overhead and '
INFO [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO [alembic.runtime.migration] Will assume transactional DDL.
INFO [ca442ccae0dd__py] returning a logger set to level: INFO for module: ca442ccae0dd__py
INFO [migrations134__py] returning a logger set to level: INFO for module: migrations134__py
INFO [alembic.runtime.migration] Running upgrade acf5fe06e3c5 -> ca442ccae0dd, empty message
INFO [ca442ccae0dd__py] Starting ca442ccae0dd upgrade
Traceback (most recent call last):
File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1901, in _execute_context
cursor, statement, parameters, context
File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 732, in do_execute
cursor.execute(statement, parameters)
psycopg2.errors.UniqueViolation: could not create unique index "uq_tenantconfig_tenant_id"
DETAIL: Key (tenant_id)=(*) is duplicated.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "/usr/local/bin/flask", line 8, in
sys.exit(main())
File "/usr/local/lib/python3.7/site-packages/flask/cli.py", line 967, in main
cli.main(args=sys.argv[1:], prog_name="python -m flask" if as_module else None)
File "/usr/local/lib/python3.7/site-packages/flask/cli.py", line 586, in main
return super(FlaskGroup, self).main(*args, **kwargs)
File "/usr/local/lib/python3.7/site-packages/click/core.py", line 782, in main
rv = self.invoke(ctx)
File "/usr/local/lib/python3.7/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/usr/local/lib/python3.7/site-packages/click/core.py", line 1259, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/usr/local/lib/python3.7/site-packages/click/core.py", line 1066, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/usr/local/lib/python3.7/site-packages/click/core.py", line 610, in invoke
return callback(*args, **kwargs)
File "/usr/local/lib/python3.7/site-packages/click/decorators.py", line 21, in new_func
return f(get_current_context(), *args, **kwargs)
File "/usr/local/lib/python3.7/site-packages/flask/cli.py", line 426, in decorator
return __ctx.invoke(f, *args, **kwargs)
File "/usr/local/lib/python3.7/site-packages/click/core.py", line 610, in invoke
return callback(*args, **kwargs)
File "/home/tapis/.local/lib/python3.7/site-packages/flask_migrate/cli.py", line 150, in upgrade
_upgrade(directory, revision, sql, tag, x_arg)
File "/home/tapis/.local/lib/python3.7/site-packages/flask_migrate/init.py", line 111, in wrapped
f(args, **kwargs)
File "/home/tapis/.local/lib/python3.7/site-packages/flask_migrate/init.py", line 200, in upgrade
command.upgrade(config, revision, sql=sql, tag=tag)
File "/home/tapis/.local/lib/python3.7/site-packages/alembic/command.py", line 378, in upgrade
script.run_env()
File "/home/tapis/.local/lib/python3.7/site-packages/alembic/script/base.py", line 576, in run_env
util.load_python_file(self.dir, "env.py")
File "/home/tapis/.local/lib/python3.7/site-packages/alembic/util/pyfiles.py", line 94, in load_python_file
module = load_module_py(module_id, path)
File "/home/tapis/.local/lib/python3.7/site-packages/alembic/util/pyfiles.py", line 110, in load_module_py
spec.loader.exec_module(module) # type: ignore
File "", line 728, in exec_module
File "", line 219, in call_with_frames_removed
File "migrations/env.py", line 96, in
run_migrations_online()
File "migrations/env.py", line 90, in run_migrations_online
context.run_migrations()
File "", line 8, in run_migrations
File "/home/tapis/.local/lib/python3.7/site-packages/alembic/runtime/environment.py", line 868, in run_migrations
self.get_context().run_migrations(**kw)
File "/home/tapis/.local/lib/python3.7/site-packages/alembic/runtime/migration.py", line 622, in run_migrations
step.migration_fn(**kw)
File "/home/tapis/migrations/versions/ca442ccae0dd
.py", line 61, in upgrade
conn.execute("alter table public.tenantconfig add constraint uq_tenantconfig_tenant_id unique (tenant_id)")
File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1370, in execute
future=False,
File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1676, in _exec_driver_sql
distilled_parameters,
File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1944, in execute_context
e, statement, parameters, cursor, context
File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 2125, in handle_dbapi_exception
sqlalchemy_exception, with_traceback=exc_info[2], from
=e
File "/usr/local/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 208, in raise

raise exception
File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1901, in _execute_context
cursor, statement, parameters, context
File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 732, in do_execute
cursor.execute(statement, parameters)
sqlalchemy.exc.IntegrityError: (psycopg2.errors.UniqueViolation) could not create unique index "uq_tenantconfig_tenant_id"
DETAIL: Key (tenant_id)=(
) is duplicated.

[SQL: alter table public.tenantconfig add constraint uq_tenantconfig_tenant_id unique (tenant_id)]
(Background on this error at: https://sqlalche.me/e/14/gkpj)

add support for running "dev" ldap for different sites/tenants

Currently, the "dev" ldap can only be used at tenant "dev" (in the primary site). This change is to parameterize the tenant id for which authenticator will use the dev ldap. This way, other sites can use authenticator and the dev ldap as a way to quickly start using the deployed services.

Support for running on Arm processors

When trying to start up a development stack on an Apple with the new Arm processors, we get an error like the following:

authenticator-ldap The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested

Since the LDAP container doesn't start, the entire stack fails, but it is possible that we would get this error for other authenticator images (and other Tapis images, for that matter) as well.

Docker supports multi-platform images, and according to the docs, building support for other platforms can be donw with buildkit without needing access to the actual hardware. See: https://docs.docker.com/build/building/multi-platform/

Token Revocation

In order to support long-lived tokens and for other reasons, we want to be able to revoke or invalidate individual JWTs. One approach to the implementation is to have a new endpoint,
DELETE /v3/tokens/
which would add the token to a persistent data store of "revoked tokens". This in turn could be implemented with a fast in-memory system like Redis, and the row representing a revoked token could be set to expire whenever the token itself expired (because if the token is expired it won't pass validation anyway). This will prevent the table from becoming too big over time.

Since services are responsible for validating tokens directly, it might be ideal to put the check to see if a token has been revoked in the nginx layer. This would prevent every service from having to implement the check. There seems to be some support in nginx for validating JWT, see https://www.nginx.com/blog/validating-oauth-2-0-access-tokens-nginx/

Federated identity prototyping with Globus

Prototype different approaches to federated identity support using Globus. Capabilities to prototype and verify include:

  • Can we filter authentication based on specific whitelisted institutions
  • Can we filter authentication based on specific whitelisted users
  • Can we check mfa status on any authentication

The first prototyping effort will use Globus tokens and APIs directly. Once we confirm the capabilities using the direct Globus approach, we will weigh the pros and cons of using keycloak as an intermediary to Globus.

Add support for TACC's Keycloak instance

This feature would add support for authenticating with TACC's Keycloak instance (identity.tacc.cloud) as a custom OAuth2 extension, analogous to the support we have for Github and the CII custom OAuth2 server. Two major benefits could be realized by this feature:

  1. Tapis can support federated login with many campus/institutional identifiers with identity.tacc.cloud's support for Globus Auth.
  2. Tapis can leverage identity mappings/reconciliation features of keycloak.

Benefit 1) should be available today, while benefit 2) may require some additional work on the identity.tacc.cloud server.

Evaluate Auth Approval Form

Overview

The token auth approval after login is unexpected for Core Portal user. Is it necessary? If so, then why so many steps (checkbox and button)?

To submit login, and then be asked to do more, is confusing. In typical user interfaces, after username and password are accepted, login is complete. But at TAPIS, it is apparently not.

As a user, if I enter my username and password, and press login, I want to login. If a token is required to do that, why does user care? If user does not approve, then they cannot login. And they only submitted login form in order to login. And after submission, think they are done, yet they are not.

Please help me understand. There may be a way to simplify this user experience, but I am not confident to propose changes until I understand the reason for the current experience.

prod. dev.
authorize prod authorize dev

🆕 Proposal

Add the branding at the top of the form (to increase trust) and change layout to:

{% if device_code %} no device_code
has device no device

Support device flow grant type with (optional) long-lived tokens

We should be able to also support a device flow for CLIs and other “headless” Tapis client apps (e.g., a Jupyter notebook). Note that we could even add support for this in place of the password grant type for tenants who use an LDAP IdP. The advantage is that the client application would not come in contact with the user’s password.

At a high level, the device flow consists of the following steps:

  1. Tapis client app requests device and user verification codes from the Tapis authorization server, and the Tapis authorization server responds with the authorization URL where the user will enter the user verification code. This is a POST request to a specific URL (<base_url>/v3/oauth2/device/code) that includes the client ID. The full response includes:
    a. device_code: a one-time use code attached to the client ID. Used for polling to retrieve the user token (step 5).
    b. user_code: the code the user enters into the web app (step 4).
    c. verification_uri: the Tapis URL the user should use to enter the user_code (step 4).
    d. expires_in: seconds when the device and user code expire.

  2. The Tapis client app prompts the user to enter a user verification code at the URL returned from step 1. The user opens a browser and points to that URL.

  3. At that Tapis URL, the first step is to authenticate the user (if they don't already have a session). This could be via a login page in the case of LDAP or Tapis could walk steps 2,3 and 4 in the case of an OAuth2 backing IdP.

  4. Once the user is authenticated, Tapis authorization server prompts the user to enter the user code from step 1.

  5. Meanwhile, the Tapis client app polls Tapis for the user authentication status and token. It does this by making a POST <base_url>/v3/oauth2/token, passing in the client id and the device_code (from step 1).
    Once the user has authenticated, entered the user code, and (optionally) authorized the device, Tapis will respond with an access token.

For more information, see the device flow spec or read about the github implementation.

Why Is Logout Done via a Form?

Overview

  1. Why is the login logout field done via a form?
  2. Can logout technically be an even that occurs upon visiting a URL?
    (So that after clicking a link or button, logout just happens.)
  3. If it must be a form, may it change
    • from [✓] Logout and [ Submit ]
    • to just [ Log out ]?

Add "admin" user to dev tenant

In addition to the testuser accounts, it would be nice to add an account called "admin" to the dev tenant by default, for testing "tenant admin" functions.

The device_code grant erroneously requires client credentials on token endpoint

When generating a token as part of the device_code grant type (i.e., the last step) the client id and key are both required: if we try a call like this:
$ curl -H "content-type: application/json" -d '{"grant_type": "device_code", "device_code": "some_code"}' https://icicleai.tapis.io/v3/oauth2/tokens
we get an error like this:
{"message":"Invalid client credentials: None, None. session: <SecureCookieSession {}>","metadata":{},"result":null,"status":"error","version":"dev"}
The whole point of device code is that it can replace password grant for CLIs and other "headless" apps, so this needs to be fixed.

Custom branding per tenant

Allow tenants to add custom branding for the various OAuth server pages such as login, authorization, etc.

Support non-standard OAuth (CII)

The CII OAuth server does not do a standard OAuth flow: in particular, 1) it does not utilize client keys and secrets and 2) it does not use an authorization code that gets exchanged for a token. In addition, the CII server returns a JWT which should be decoded to get the user's identity. This extension makes customizations to the authenticator's OAuth2-provider support to support CII's non-standard OAuth implementation.

The tasks in this issue are:

  • update the custom_idp_config_schema with a "ciiObject" description; properties needed include login_url and jwt_decode_key, and update the code to recognize this new schema type (e.g., in TenantConfigsCache)
  • Update the oauth2ext.py module to support the new cii type (instructions at top of module).
  • modify the OAuth2ProviderExtCallback controller class to not try to generate an authorization code.
  • [ ]

Update Tenant Form Design?

Overview

The tenant form (dev) does not use new design.

Screenshot Screenshot 2023-09-12 at 1 55 29 PM

What I Expected

Given the update of the other forms, this form seems to merit the same update.

What I Propose

  1. Update the tenant form to use the new design.

    Title
    brief description of action

    Select tenant:
    [________________ ▾ ]

  2. Consider whether to use <select> or many <button>s:

    Title
    brief description of action

    [ A2CPS Tenant ]
    [ Dev Tenant ]
    [ Icicle Tenant ]

Make `tenants` config optional

Currently, the tenants config is required which means that to configure an authenticator, an administrator must know all the tenants. Let's change the config to optional with the behavior that, when not set, the authenticator assumes it will serve all tenants for the site it runs in.

Change MFA default bevior to required on every login

Currently, once a user has passed an MFA challenge, this fact is cached in the session and the user is not required to pass MFA again until the session expires or the user logs out, etc. We should change the default behavior to require an MFA on every login.

IDP Form Does Not Give User Context

Overview

The IPD form (that I have a URL to) does not give user ample context to confidently make a decision.

What I Know

After chatting with the dev team, I understand this form is how the user chooses by what means they wish to authenticate. A typical user will see options "TACC Accounts" and "Login with Globus". A dev site user will see more options.

dev account options better ui

Basically this is like when signing in with Zoom, you have several ways to auth, and Zoom doesn't know which is suitable to your situation. But, unlike Zoom, there is no direct user account. This is the "or sign in with" and only that.

zoom or sign in with

What I Expect

Context. Why am I making a choice? And what choice am I making?

What I Propose

Important
I am still learning TAPIS, so I can't offer anything definitive nor with any authority. There may be a semantic or technical problem with my proposals. Please share concerns, questions, etc. DesignSafe UI designer C.J. is helping me out.

New Form Description

Log in to TAPIS
Select an account to continue

New Interaction Method & Layout

Instead of several radio buttons, have a button for each, (ideally) with an icon.

tapis login buttons v4

Technical Direction

  • The button icons would be c-button__icon--before (see "Icons"). This may required loading COre-Styles components/c-button.html.
  • The buttons I mocked up used min-width: 300px.
  • To submit the values with different buttons, but have unique user-facing text, use <button type="submit" value="…">Some Text</button> (source).

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.