Skip to content

Commit

Permalink
Persist credential with typed values (#1530)
Browse files Browse the repository at this point in the history
* Insert credential body with properfield types

* Add googlesheets schema for tests

* Simplify params

* Reuse change_credential

* Apply typed body to update credentials

* Cast credential body on worker fetch credential

* Add changelog

* Handle raw schema and unchanged schema on update

* Add migration to update credential

* Don't cast existing credentials on fetch credential

* Add update_credential

* Use testable migration

* Fix warning

* Add explicit test for returning saved credential

* Add create test to check same cast as migrate

* Liveview test creating a credential

* Raw body updated with an integer
  • Loading branch information
jyeshe authored Dec 7, 2023
1 parent 8bcd3c3 commit 54b50bb
Show file tree
Hide file tree
Showing 12 changed files with 608 additions and 187 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ and this project adheres to
[#1508](https://github.com/OpenFn/Lightning/issues/1508)
- Enable user to reauthorize and obtain a new refresh token.
[#1495](https://github.com/OpenFn/Lightning/issues/1495)
- Save credential body with types declared on schema
[#1518](https://github.com/OpenFn/Lightning/issues/1518)

## [v0.10.5] - 2023-12-03

Expand Down
70 changes: 63 additions & 7 deletions lib/lightning/credentials.ex
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,14 @@ defmodule Lightning.Credentials do
alias Lightning.Repo
alias Ecto.Multi

alias Lightning.Credentials.{Audit, Credential, SensitiveValues}
alias Lightning.Credentials.Audit
alias Lightning.Credentials.Credential
alias Lightning.Credentials.SchemaDocument
alias Lightning.Credentials.SensitiveValues
alias Lightning.Projects.Project

require Logger

@doc """
Perform, when called with %{"type" => "purge_deleted"}
will find credentials that are ready for permanent deletion, set their bodies to null, and attempt to purge them.
Expand Down Expand Up @@ -116,17 +121,16 @@ defmodule Lightning.Credentials do
"""
def create_credential(attrs \\ %{}) do
changeset = %Credential{} |> change_credential(attrs) |> cast_body_change()

Multi.new()
|> Multi.insert(
:credential,
Credential.changeset(%Credential{}, attrs |> coerce_json_field("body"))
)
|> Multi.insert(:credential, changeset)
|> Multi.insert(:audit, fn %{credential: credential} ->
Audit.event("created", credential.id, credential.user_id)
end)
|> Repo.transaction()
|> case do
{:error, :credential, changeset, _changes} ->
{:error, _op, changeset, _changes} ->
{:error, changeset}

{:ok, %{credential: credential}} ->
Expand All @@ -147,7 +151,7 @@ defmodule Lightning.Credentials do
"""
def update_credential(%Credential{} = credential, attrs) do
changeset = change_credential(credential, attrs)
changeset = credential |> change_credential(attrs) |> cast_body_change()

Multi.new()
|> Multi.update(:credential, changeset)
Expand Down Expand Up @@ -181,6 +185,58 @@ defmodule Lightning.Credentials do
end
end

# Migration only
def migrate_credential_body(
%Credential{body: body, schema: schema_name} = credential
) do
case put_typed_body(body, schema_name) do
{:ok, ^body} ->
:ok

{:ok, changed_body} ->
credential
|> change_credential(%{body: changed_body})
|> Repo.update!()

{:error, %Ecto.Changeset{errors: errors}} ->
Logger.warning(fn ->
"Casting credential on migration failed with reason: #{inspect(errors)}"
end)
end
end

defp cast_body_change(
%Ecto.Changeset{valid?: true, changes: %{body: body}} = changeset
) do
schema_name = Ecto.Changeset.get_field(changeset, :schema)

case put_typed_body(body, schema_name) do
{:ok, updated_body} ->
Ecto.Changeset.put_change(changeset, :body, updated_body)

{:error, _reason} ->
Ecto.Changeset.add_error(changeset, :body, "Invalid body types")
end
end

defp cast_body_change(changeset), do: changeset

defp put_typed_body(body, "raw"), do: {:ok, body}

defp put_typed_body(body, schema_name) do
schema = get_schema(schema_name)

with changeset <- SchemaDocument.changeset(body, schema: schema),
{:ok, typed_body} <- Ecto.Changeset.apply_action(changeset, :insert) do
updated_body =
Enum.into(typed_body, body, fn {field, typed_value} ->
{to_string(field), typed_value}
end)

{:ok, updated_body}
end
end

defp derive_events(
multi,
%Ecto.Changeset{data: %Credential{}} = changeset
Expand Down
2 changes: 1 addition & 1 deletion lib/lightning/credentials/schema_document.ex
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ defmodule Lightning.Credentials.SchemaDocument do
alias Lightning.Credentials.Schema
import Ecto.Changeset

def changeset(document, attrs, schema: schema = %Schema{}) do
def changeset(document \\ %{}, attrs, schema: schema = %Schema{}) do
{document, schema.types}
|> cast(attrs, schema.fields)
|> Schema.validate(schema)
Expand Down
10 changes: 6 additions & 4 deletions lib/lightning_web/live/credential_live/form_component.ex
Original file line number Diff line number Diff line change
Expand Up @@ -465,17 +465,19 @@ defmodule LightningWeb.CredentialLive.FormComponent do
end

defp save_credential(socket, :new, credential_params) do
user_id = Ecto.Changeset.fetch_field!(socket.assigns.changeset, :user_id)
%{changeset: changeset, type: schema_name} = socket.assigns

user_id = Ecto.Changeset.fetch_field!(changeset, :user_id)

project_credentials =
Ecto.Changeset.fetch_field!(socket.assigns.changeset, :project_credentials)
Ecto.Changeset.fetch_field!(changeset, :project_credentials)
|> Enum.map(fn %{project_id: project_id} ->
%{"project_id" => project_id}
end)

credential_params
|> Map.put("user_id", user_id)
|> Map.put("schema", socket.assigns.type)
|> Map.put("schema", schema_name)
|> Map.put("project_credentials", project_credentials)
|> Credentials.create_credential()
|> case do
Expand All @@ -491,7 +493,7 @@ defmodule LightningWeb.CredentialLive.FormComponent do
end

{:error, %Ecto.Changeset{} = changeset} ->
{:noreply, assign(socket, changeset: changeset)}
{:noreply, assign(socket, :changeset, changeset)}
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ defmodule LightningWeb.CredentialLive.JsonSchemaBodyComponent do
end

defp create_schema_changeset(schema, params) do
Credentials.SchemaDocument.changeset(%{}, params, schema: schema)
Credentials.SchemaDocument.changeset(params, schema: schema)
end

attr :form, :map, required: true
Expand Down
18 changes: 18 additions & 0 deletions priv/repo/migrations/20231206115145_cast_credential_body.exs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
defmodule Lightning.Repo.Migrations.CastCredentialBody do
use Ecto.Migration

alias Lightning.Credentials
alias Lightning.Credentials.Credential
alias Lightning.Repo

def change do
[{Lightning.Vault, Application.get_env(:lightning, Lightning.Vault, [])}]
|> Supervisor.start_link(strategy: :one_for_one)

Repo.transaction(fn ->
Credential
|> Repo.all()
|> Enum.each(&Credentials.migrate_credential_body/1)
end)
end
end
19 changes: 19 additions & 0 deletions test/fixtures/schemas/googlesheets.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"$comment": "OAuth2",
"properties": {
"access_token": {
"title": "Access Token",
"type": "string",
"description": "Your Google Sheets access token",
"writeOnly": true,
"minLength": 1,
"examples": [
"eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiIsIng1dCI6IjlGWERwYmZNRlQyU3ZRdVhoODQ2WVR3RUlCdyIsI"
]
}
},
"type": "object",
"additionalProperties": true,
"required": ["access_token"]
}
3 changes: 1 addition & 2 deletions test/lightning/credentials/schema_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ defmodule Lightning.Credentials.SchemaTest do

test "can ", %{schema: schema} do
changeset =
SchemaDocument.changeset(%{}, %{"foo" => "bar", "password" => "pass"},
SchemaDocument.changeset(%{"foo" => "bar", "password" => "pass"},
schema: schema
)

Expand All @@ -155,7 +155,6 @@ defmodule Lightning.Credentials.SchemaTest do

changeset =
SchemaDocument.changeset(
%{},
%{
"username" => "initial",
"password" => "pass",
Expand Down
Loading

0 comments on commit 54b50bb

Please sign in to comment.