Skip to content

Commit

Permalink
tailwind-ize and dry up the alerts for oauth credentials (OpenFn#2314)
Browse files Browse the repository at this point in the history
* Add support for revoking token before reauthorization

* use names from Oauth2 specification

* use 'here'

* use <.icon />, not <Heroicons />

* revoke oauth access on delete

* rename migration

* update CL

* Refactor and introduce revoke failed feedback

* Fix failing tests

* Add forgotten asserts for oauth error components

* Test schedule_credential_deletion/1 revokes token for oauth credentials

* Assert event is pushed to hook when authorize url is clicked

* More tests for failure cases

* Receive authorize_url after it has been sent to hook

* Remove unnecessary code and add todo for next iterations

* Reduce todo string length

* Make sure we don't present re-authorization button when revocation is not available

* When deleting an oauth credential, let's make sure a revocation endpoint is available before we attempt to revoke the token

* After rebase and fixing tests

* Test success_message when revocation is available and also when it is unavailable

* wip

* dry up the code

* more

* fix up tests

* cleanup more tailwind

* fix run link

* introduce link-plain

* link style

* don't save when clicking reauthorize

* type button for userinfo also

* danger

* language for reauthenticate

* fix tests

* update CL

* try userinfo again

* Fetch userinfo loader

* call it an alert_block

* don't revoke before requesting new scopes

* onre more concept

* merge token with previous one

* revoke if possible

* language

* more testing

* losing my mind, why don't scopes get updated?

* allow revoke and retry

* don't update credential before calling update_credential

* fix tests

* Make sure we get values from changeset

* Remove IO.inspects

* Refactor validate to work with changeset and pipe all required params

* final test

---------

Co-authored-by: Elias W. BA <[email protected]>
  • Loading branch information
taylordowns2000 and elias-ba committed Jul 24, 2024
1 parent 3bbc04c commit 294a539
Show file tree
Hide file tree
Showing 41 changed files with 888 additions and 345 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ and this project adheres to

- Enable End to End Integration tests
[#2187](https://github.com/OpenFn/lightning/issues/2187)
- Use the Oauth2 `revocation_endpoint` to revoke token access (1) before
attempting to reauthorize and (2) when users schedule a credential for
deletion [#2314](https://github.com/OpenFn/lightning/issues/2314)
- Standardized tailwind alerts
[#2314](https://github.com/OpenFn/lightning/issues/2314)
- Standardized `link` tailwind style (and provided `link-plain`, `link-info`,
`link-error`, and `link-warning`)
[#2314](https://github.com/OpenFn/lightning/issues/2314)

### Fixed

Expand Down
34 changes: 34 additions & 0 deletions assets/css/app.css
Original file line number Diff line number Diff line change
Expand Up @@ -278,3 +278,37 @@ div.line-num::before {
}
}
}

@layer components {
.link {
@apply cursor-pointer
/* underline */
underline-offset-2
hover:underline hover:underline-offset-2
whitespace-nowrap
inline-block
font-medium
text-indigo-400 hover:text-indigo-500;
}

.link-plain {
@apply font-normal
text-gray-800;
}

.link-info {
@apply text-blue-700 hover:text-blue-600;
}

.link-danger {
@apply text-red-700 hover:text-red-600;
}

.link-success {
@apply text-green-700 hover:text-green-600;
}

.link-warning {
@apply text-yellow-700 hover:text-yellow-600;
}
}
4 changes: 2 additions & 2 deletions assets/js/adaptor-docs/components/DocsPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const DocsLink = ({ specifier }: { specifier: string }) => {
<div>
External docs:
<a
className="text-indigo-400 underline underline-offset-2 hover:text-indigo-500 ml-2"
className="link ml-2"
href={`https://docs.openfn.org/adaptors/packages/${name}-docs`}
target="_blank"
>
Expand All @@ -32,7 +32,7 @@ const DocsLink = ({ specifier }: { specifier: string }) => {
<div>
Source code:
<a
className="text-indigo-400 underline underline-offset-2 hover:text-indigo-500 ml-2"
className="link ml-2"
href={srcLink}
target="_blank"
>
Expand Down
8 changes: 8 additions & 0 deletions assets/js/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,14 @@ export {
TabbedPanels,
};

export const OpenAuthorizeUrl = {
mounted() {
this.handleEvent('open_authorize_url', ({ url }: { url: string }) => {
window.open(url, '_blank');
});
},
} as PhoenixHook;

export const EditScope = {
mounted() {
this.el.addEventListener('dblclick', _e => {
Expand Down
58 changes: 47 additions & 11 deletions lib/lightning/auth_providers/oauth_http_client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,32 @@ defmodule Lightning.AuthProviders.OauthHTTPClient do

alias LightningWeb.RouteHelpers

@doc """
Revokes an OAuth token.
## Parameters
- `client`: The client configuration containing client_id and client_secret.
- `token`: The token to be revoked (access_token or refresh_token).
## Returns
- `{:ok, response}` on success
- `{:error, reason}` on failure
"""
def revoke_token(client, token) do
body = %{
token: token["access_token"],
client_id: client.client_id,
client_secret: client.client_secret
}

Tesla.client([
Tesla.Middleware.FormUrlencoded,
Tesla.Middleware.FollowRedirects
])
|> post(client.revocation_endpoint, body)
|> handle_resp([200])
end

@doc """
Fetches a new token using the authorization code provided by the OAuth provider.
Expand Down Expand Up @@ -179,17 +205,27 @@ defmodule Lightning.AuthProviders.OauthHTTPClient do
|> handle_resp([200])
end

defp handle_resp(result, success_statuses) do
case result do
{:ok, %Tesla.Env{status: status, body: body}} ->
if status in success_statuses do
Jason.decode(body)
else
{:error, "#{inspect(body)}"}
end

{:error, reason} ->
{:error, "#{inspect(reason)}"}
defp handle_resp(
{:ok, %Tesla.Env{status: status, body: body}},
expected_statuses
) do
if status in expected_statuses do
case body do
"" ->
{:ok, %{}}

_ ->
case Jason.decode(body) do
{:ok, decoded_body} -> {:ok, decoded_body}
{:error, reason} -> {:error, reason}
end
end
else
{:error, inspect(body)}
end
end

defp handle_resp({:error, reason}, _expected_statuses) do
{:error, inspect(reason)}
end
end
23 changes: 22 additions & 1 deletion lib/lightning/credentials.ex
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ defmodule Lightning.Credentials do
alias Lightning.Credentials
alias Lightning.Credentials.Audit
alias Lightning.Credentials.Credential
alias Lightning.Credentials.OauthClient
alias Lightning.Credentials.SchemaDocument
alias Lightning.Credentials.SensitiveValues
alias Lightning.Projects.Project
Expand Down Expand Up @@ -174,7 +175,10 @@ defmodule Lightning.Credentials do
"""
def update_credential(%Credential{} = credential, attrs) do
changeset = credential |> change_credential(attrs) |> cast_body_change()
changeset =
credential
|> change_credential(attrs)
|> cast_body_change()

Multi.new()
|> Multi.update(:credential, changeset)
Expand All @@ -185,6 +189,9 @@ defmodule Lightning.Credentials do
{:error, changeset}

{:ok, %{credential: credential}} ->
Lightning.Repo.get(Lightning.Credentials.Credential, credential.id)
|> Map.get(:body)

{:ok, credential}
end
end
Expand Down Expand Up @@ -390,6 +397,7 @@ defmodule Lightning.Credentials do
case Repo.update(changeset) do
{:ok, updated_credential} ->
remove_credential_associations(updated_credential)
maybe_revoke_oauth(updated_credential)
notify_owner(updated_credential)
{:ok, updated_credential}

Expand All @@ -410,6 +418,19 @@ defmodule Lightning.Credentials do
})
end

defp maybe_revoke_oauth(%Credential{oauth_client_id: nil}), do: :ok

defp maybe_revoke_oauth(%Credential{
oauth_client_id: oauth_client_id,
body: body
}) do
client = Repo.get(OauthClient, oauth_client_id)

if client.revocation_endpoint do
OauthHTTPClient.revoke_token(client, body)
end
end

defp remove_credential_associations(%Credential{id: credential_id}) do
project_credential_ids_query =
from(pc in Lightning.Projects.ProjectCredential,
Expand Down
3 changes: 3 additions & 0 deletions lib/lightning/credentials/oauth_client.ex
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ defmodule Lightning.Credentials.OauthClient do
field :client_secret, :string
field :authorization_endpoint, :string
field :token_endpoint, :string
field :revocation_endpoint, :string
field :userinfo_endpoint, :string
field :introspection_endpoint, :string
field :global, :boolean, default: false
Expand Down Expand Up @@ -62,6 +63,7 @@ defmodule Lightning.Credentials.OauthClient do
:client_secret,
:authorization_endpoint,
:token_endpoint,
:revocation_endpoint,
:userinfo_endpoint,
:introspection_endpoint,
:global,
Expand All @@ -79,6 +81,7 @@ defmodule Lightning.Credentials.OauthClient do
])
|> Validators.validate_url(:authorization_endpoint)
|> Validators.validate_url(:token_endpoint)
|> Validators.validate_url(:revocation_endpoint)
|> Validators.validate_url(:userinfo_endpoint)
|> Validators.validate_url(:introspection_endpoint)
|> Validators.validate_url(:scopes_doc_url)
Expand Down
2 changes: 1 addition & 1 deletion lib/lightning/oauth_clients.ex
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ defmodule Lightning.OauthClients do
## Parameters
- attrs: Map containing attributes for the new OAuth client.
Required fields: `name`, `authorization_endpoint`, `token_endpoint`, `client_id`, `client_secret`.
Required fields: `name`, `authorization_endpoint`, `token_endpoint`, `revocation_endpoint`, `client_id`, `client_secret`.
## Returns
- `{:ok, oauth_client}` if the client is created successfully.
Expand Down
4 changes: 2 additions & 2 deletions lib/lightning_web/components/viewers.ex
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ defmodule LightningWeb.Components.Viewers do
You can’t rerun this work order, but you can change
<.link
navigate={~p"/projects/#{@project_id}/settings#data-storage"}
class="underline inline-block text-blue-400 hover:text-blue-600"
class="link"
>
this policy
</.link>
Expand All @@ -217,7 +217,7 @@ defmodule LightningWeb.Components.Viewers do
<span
id="zero-persistence-admins-tooltip"
phx-hook="Tooltip"
class="underline inline-block text-blue-400"
class="link inline-block"
aria-label={Enum.join(@admin_contacts, ", ")}
>
project admins
Expand Down
6 changes: 3 additions & 3 deletions lib/lightning_web/controllers/user_auth.ex
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ defmodule LightningWeb.UserAuth do
end

@doc """
Re-Authenticates the user by using the sudo token
Reauthenticate the user by using the sudo token
"""
def reauth_sudo_mode(conn, _opts) do
conn = ensure_sudo_token(conn)
Expand Down Expand Up @@ -267,7 +267,7 @@ defmodule LightningWeb.UserAuth do
end

@doc """
Used for routes that require the user to be re-authenticated.
Used for routes that require the user to be reauthenticated.
"""
def require_sudo_user(conn, _opts) do
if conn.assigns[:sudo_mode?] do
Expand Down Expand Up @@ -313,7 +313,7 @@ defmodule LightningWeb.UserAuth do
end

@doc """
Used for LiveView routes that require the user to be re-authenticated.
Used for LiveView routes that require the user to be reauthenticated.
"""
def on_mount(:ensure_sudo, _params, session, socket) do
socket = mount_sudo_mode(session, socket)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
ensure that you understand, accept, and are willing to abide
by all of their terms and conditions. You can read the
OpenFn.org terms of service <.link
class="text-indigo-600 hover:underline"
class="link"
target="_blank"
href="https://www.openfn.org/terms"
>here</.link>.
Expand Down
2 changes: 1 addition & 1 deletion lib/lightning_web/controllers/user_totp_html/new.html.heex
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
href={
~p"/users/two-factor?authentication_type=#{invert_chosen_type(@authentication_type)}&user[remember_me]=#{@remember_me || false}"
}
class="font-medium text-blue-600 dark:text-blue-500 hover:underline"
class="link font-medium"
>
<%= if @authentication_type == :totp do %>
Use your backup code
Expand Down
Loading

0 comments on commit 294a539

Please sign in to comment.