Skip to content

Commit

Permalink
Don't allow url based uploads
Browse files Browse the repository at this point in the history
This is technically somewhat insecure (not that bad, since requests are auto-ignored if not jpeg/png files).  Should also think of a way to refactor the avatar upload out of the oidc login path entirely, and move it async, just not a priority.
  • Loading branch information
michaeljguarino committed Oct 1, 2024
1 parent 368691e commit fd63e66
Show file tree
Hide file tree
Showing 11 changed files with 19 additions and 14 deletions.
2 changes: 1 addition & 1 deletion apps/core/lib/core/schema/account.ex
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ defmodule Core.Schema.Account do
|> unique_constraint(:name)
|> validate_required([:name])
|> generate_uuid(:icon_id)
|> cast_attachments(attrs, [:icon], allow_urls: true)
|> cast_attachments(attrs, [:icon])
|> set_address_updated()
|> reject_urls(:name)
end
Expand Down
4 changes: 2 additions & 2 deletions apps/core/lib/core/schema/artifact.ex
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ defmodule Core.Schema.Artifact do
|> validate_required([:name, :platform, :type, :arch])
|> foreign_key_constraint(:repository_id)
|> unique_constraint(:repository_id, name: index_name(:artifacts, [:repository_id, :name, :platform, :arch]))
|> cast_attachments(attrs, [:blob], allow_urls: true)
|> cast_attachments(attrs, [:blob])
|> add_sha(attrs)
|> add_filesize(attrs)
end
Expand All @@ -52,4 +52,4 @@ defmodule Core.Schema.Artifact do
end
end
def add_filesize(changeset, _), do: changeset
end
end
4 changes: 2 additions & 2 deletions apps/core/lib/core/schema/crd.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ defmodule Core.Schema.Crd do
|> generate_uuid(:blob_id)
|> foreign_key_constraint(:version_id)
|> unique_constraint(:name, name: index_name(:crds, [:version_id, :name]))
|> cast_attachments(attrs, [:blob], allow_urls: true)
|> cast_attachments(attrs, [:blob])
|> validate_required([:name, :version_id])
end
end
end
2 changes: 1 addition & 1 deletion apps/core/lib/core/schema/file.ex
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ defmodule Core.Schema.File do
model
|> cast(attrs, @valid)
|> generate_uuid(:blob_id)
|> cast_attachments(attrs, [:blob], allow_urls: true)
|> cast_attachments(attrs, [:blob])
|> foreign_key_constraint(:message_id)
|> put_change(:filename, filename(upload))
|> put_change(:filesize, file_size(upload))
Expand Down
4 changes: 2 additions & 2 deletions apps/core/lib/core/schema/integration.ex
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ defmodule Core.Schema.Integration do
|> foreign_key_constraint(:repository_id)
|> unique_constraint(:name, name: index_name(:integrations, [:repository_id, :name]))
|> generate_uuid(:icon_id)
|> cast_attachments(attrs, [:icon], allow_urls: true)
|> cast_attachments(attrs, [:icon])
|> validate_required([:name, :spec])
end

Expand All @@ -64,4 +64,4 @@ defmodule Core.Schema.Integration do
end)
end
def validate(changeset, _), do: add_error(changeset, :spec, "No resource definition present for this repository")
end
end
2 changes: 1 addition & 1 deletion apps/core/lib/core/schema/publisher.ex
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ defmodule Core.Schema.Publisher do
|> unique_constraint(:owner_id)
|> validate_length(:name, max: 255)
|> generate_uuid(:avatar_id)
|> cast_attachments(attrs, [:avatar], allow_urls: true)
|> cast_attachments(attrs, [:avatar])
end

@stripe_valid ~w(billing_account_id)a
Expand Down
2 changes: 1 addition & 1 deletion apps/core/lib/core/schema/repository.ex
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ defmodule Core.Schema.Repository do
|> unique_constraint(:name)
|> validate_required([:name, :category])
|> generate_uuid(:icon_id)
|> cast_attachments(attrs, [:icon, :dark_icon, :docs], allow_urls: true)
|> cast_attachments(attrs, [:icon, :dark_icon, :docs])
end

@keyvalid ~w(public_key private_key)a
Expand Down
4 changes: 2 additions & 2 deletions apps/core/lib/core/schema/terraform.ex
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ defmodule Core.Schema.Terraform do
|> unique_constraint(:name, name: index_name(:terraform, [:repository_id, :name]))
|> foreign_key_constraint(:repository_id)
|> bump_version(schema)
|> cast_attachments(attrs, [:package], allow_urls: true)
|> cast_attachments(attrs, [:package])
|> validate_required([:name, :repository_id])
end

Expand All @@ -62,4 +62,4 @@ defmodule Core.Schema.Terraform do
_ -> cs
end
end
end
end
6 changes: 5 additions & 1 deletion apps/core/lib/core/schema/user.ex
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ defmodule Core.Schema.User do
timestamps()
end

def mark_url_safe(), do: Process.put({__MODULE__, :url_safe}, true)

def url_safe?(), do: !!Process.get({__MODULE__, :url_safe})

def intercom_id(%__MODULE__{id: id}), do: Core.sha("#{id}:#{Core.conf(:intercom_salt)}")

def service_account(query \\ __MODULE__, is_svc \\ :yes)
Expand Down Expand Up @@ -209,7 +213,7 @@ defmodule Core.Schema.User do
|> hash_password()
|> generate_uuid(:avatar_id)
|> change_markers(password_hash: :password_change)
|> cast_attachments(attrs, [:avatar], allow_urls: true)
|> cast_attachments(attrs, [:avatar], (if url_safe?(), do: [allow_urls: true], else: []))
|> set_email_changed()
|> reject_passwordless()
end
Expand Down
2 changes: 1 addition & 1 deletion apps/core/lib/core/schema/version.ex
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ defmodule Core.Schema.Version do
|> cast_assoc(:tags)
|> validate_required([:version])
|> generate_uuid(:package_id)
|> cast_attachments(attrs, [:package], allow_urls: true)
|> cast_attachments(attrs, [:package])
|> unique_constraint(:chart_id, name: index_name(:versions, [:chart_id, :version]))
|> unique_constraint(:terraform_id, name: index_name(:versions, [:terraform_id, :version]))
|> foreign_key_constraint(:chart_id)
Expand Down
1 change: 1 addition & 0 deletions apps/core/lib/core/services/users.ex
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ defmodule Core.Services.Users do
"""
@spec bootstrap_user(Core.OAuth.method, map) :: user_resp
def bootstrap_user(service, %{email: email} = attrs) do
User.mark_url_safe()
case {service, get_user_by_email(email)} do
{_, nil} ->
attrs
Expand Down

0 comments on commit fd63e66

Please sign in to comment.