Merge branch 'main' into feat/service-accounts

Author: skipi

artifacthub/pkg/server/public/publicserver.go

@@ -78,8 +78,6 @@ func getAuthTokenFromContext(ctx context.Context) (string, error) {
 // artifact storage, and deleting as well.
 func (s *Server) GenerateSignedURLs(ctx context.Context,
 	q *artifacts.GenerateSignedURLsRequest) (*artifacts.GenerateSignedURLsResponse, error) {
-	log.Info("[GenerateSignedURLs] Received", zap.Reflect("request", q))
-
 	response := &artifacts.GenerateSignedURLsResponse{}
 	token, err := getAuthTokenFromContext(ctx)
 	if err != nil {
@@ -93,12 +91,22 @@ func (s *Server) GenerateSignedURLs(ctx context.Context,
 		return response, nil
 	}
 
-	artifact, err := s.authenticate(token, q.Paths)
+	artifact, claims, err := s.authenticateAndGetClaims(token, q.Paths)
 	if err != nil {
 		log.Error("Error authenticating request", zap.Error(err))
 		return nil, err
 	}
 
+	log.Info("[GenerateSignedURLs] Authenticated request",
+		zap.String("type", q.Type.String()),
+		zap.Int("paths_count", len(q.Paths)),
+		zap.Strings("paths", q.Paths),
+		zap.String("artifact", claims.ArtifactID),
+		zap.String("project", claims.Project),
+		zap.String("job", claims.Job),
+		zap.String("workflow", claims.Workflow),
+	)
+
 	var us []*artifacts.SignedURL
 	switch q.Type {
 	case artifacts.GenerateSignedURLsRequest_PUSH:
@@ -177,26 +185,23 @@ func getMaxReceiveMessageSize() int {
 	return maxReceiveMsgSize
 }
 
-func (s *Server) authenticate(token string, paths []string) (*models.Artifact, error) {
+func (s *Server) authenticateAndGetClaims(token string, paths []string) (*models.Artifact, *jwt.Claims, error) {
 	resourceType, resourceID, err := s.findAndValidateResource(paths)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	claims, err := s.validateJWT(resourceType, resourceID, token)
 	if err != nil {
-		return nil, status.Error(codes.PermissionDenied, err.Error())
+		return nil, nil, status.Error(codes.PermissionDenied, err.Error())
 	}
 
-	log.Info(
-		"Granted access to artifact storage through JWT",
-		zap.String("artifact", claims.ArtifactID),
-		zap.String("project", claims.Project),
-		zap.String("job", claims.Job),
-		zap.String("workflow", claims.Workflow),
-	)
+	artifacts, err := models.FindArtifactByID(claims.ArtifactID)
+	if err != nil {
+		return nil, nil, err
+	}
 
-	return models.FindArtifactByID(claims.ArtifactID)
+	return artifacts, claims, nil
 }
 
 func (s *Server) validateJWT(resourceType, resourceID, token string) (*jwt.Claims, error) {

auth/lib/auth.ex

@@ -115,7 +115,7 @@ defmodule Auth do
       !org ->
         send_resp(conn, 401, "Unauthorized")
 
-      Auth.IpFilter.block?(conn, org) ->
+      Auth.IpFilter.block?(conn.remote_ip, org) ->
         send_resp(conn, 404, blocked_ip_response(conn))
 
       true ->
@@ -326,7 +326,7 @@ defmodule Auth do
       !org ->
         {:error, :missing_organization, conn}
 
-      Auth.IpFilter.block?(conn, org) ->
+      Auth.IpFilter.block?(conn.remote_ip, org) ->
         {:error, :unauthorized_ip, conn}
 
       true ->
@@ -387,7 +387,7 @@ defmodule Auth do
       !org ->
         {:error, :missing_organization, conn}
 
-      Auth.IpFilter.block?(conn, org) ->
+      Auth.IpFilter.block?(conn.remote_ip, org) ->
         {:error, :unauthorized_ip, conn}
 
       true ->
@@ -416,7 +416,7 @@ defmodule Auth do
       !org ->
         {:error, :missing_organization, conn}
 
-      Auth.IpFilter.block?(conn, org) ->
+      Auth.IpFilter.block?(conn.remote_ip, org) ->
         {:error, :unauthorized_ip, conn}
 
       true ->
@@ -680,7 +680,7 @@ defmodule Auth do
   end
 
   defp blocked_ip_response(conn) do
-    ip = Auth.IpFilter.client_ip(conn) |> Tuple.to_list() |> Enum.join(".")
+    ip = conn.remote_ip |> :inet.ntoa()
 
     """
       You cannot access this organization from your current IP address (#{ip}) due to the security settings enabled by the organization administrator.

auth/lib/auth/ip_filter.ex

@@ -1,31 +1,14 @@
 defmodule Auth.IpFilter do
   require Logger
 
-  def block?(conn, org) do
+  def block?(client_ip, org) do
     if Enum.empty?(org.ip_allow_list) do
       false
     else
-      _block?(client_ip(conn), org.ip_allow_list)
+      _block?(client_ip, org.ip_allow_list)
     end
   end
 
-  def client_ip(conn) do
-    Plug.Conn.get_req_header(conn, "x-forwarded-for")
-    |> List.last()
-    |> String.split(", ")
-    |> List.first()
-    |> InetCidr.parse_address!()
-  rescue
-    e ->
-      Watchman.increment("auth.ip_filter.error")
-      Logger.error("Error parsing client IP: #{inspect(e)}")
-      Logger.error("Headers: #{inspect(conn.req_headers)}")
-
-      # If something goes wrong here, it means Ingress/Ambassador are sending us
-      # a bad X-Forwarded-For header, which is very unlikely, so we fail open
-      nil
-  end
-
   defp _block?(nil, _), do: false
 
   defp _block?(client_ip, ip_allow_list) do

auth/mix.lock

@@ -21,19 +21,19 @@
   "jumper": {:hex, :jumper, "1.0.1", "3c00542ef1a83532b72269fab9f0f0c82bf23a35e27d278bfd9ed0865cecabff", [:mix], [], "hexpm", "318c59078ac220e966d27af3646026db9b5a5e6703cb2aa3e26bcfaba65b7433"},
   "junit_formatter": {:hex, :junit_formatter, "3.3.1", "c729befb848f1b9571f317d2fefa648e9d4869befc4b2980daca7c1edc468e40", [:mix], [], "hexpm", "761fc5be4b4c15d8ba91a6dafde0b2c2ae6db9da7b8832a55b5a1deb524da72b"},
   "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
-  "mime": {:hex, :mime, "2.0.5", "dc34c8efd439abe6ae0343edbb8556f4d63f178594894720607772a041b04b02", [:mix], [], "hexpm", "da0d64a365c45bc9935cc5c8a7fc5e49a0e0f9932a761c55d6c52b142780a05c"},
+  "mime": {:hex, :mime, "2.0.7", "b8d739037be7cd402aee1ba0306edfdef982687ee7e9859bee6198c1e7e2f128", [:mix], [], "hexpm", "6171188e399ee16023ffc5b76ce445eb6d9672e2e241d2df6050f3c771e80ccd"},
   "mimerl": {:hex, :mimerl, "1.3.0", "d0cd9fc04b9061f82490f6581e0128379830e78535e017f7780f37fea7545726", [:rebar3], [], "hexpm", "a1e15a50d1887217de95f0b9b0793e32853f7c258a5cd227650889b38839fe9d"},
   "parse_trans": {:hex, :parse_trans, "3.4.1", "6e6aa8167cb44cc8f39441d05193be6e6f4e7c2946cb2759f015f8c56b76e5ff", [:rebar3], [], "hexpm", "620a406ce75dada827b82e453c19cf06776be266f5a67cff34e1ef2cbb60e49a"},
-  "plug": {:hex, :plug, "1.14.2", "cff7d4ec45b4ae176a227acd94a7ab536d9b37b942c8e8fa6dfc0fff98ff4d80", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "842fc50187e13cf4ac3b253d47d9474ed6c296a8732752835ce4a86acdf68d13"},
+  "plug": {:hex, :plug, "1.18.1", "5067f26f7745b7e31bc3368bc1a2b818b9779faa959b49c934c17730efc911cf", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "57a57db70df2b422b564437d2d33cf8d33cd16339c1edb190cd11b1a3a546cc2"},
   "plug_cowboy": {:hex, :plug_cowboy, "2.5.2", "62894ccd601cf9597e2c23911ff12798a8a18d237e9739f58a6b04e4988899fe", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:cowboy_telemetry, "~> 0.3", [hex: :cowboy_telemetry, repo: "hexpm", optional: false]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "ea6e87f774c8608d60c8d34022a7d073bd7680a0a013f049fc62bf35efea1044"},
-  "plug_crypto": {:hex, :plug_crypto, "1.2.5", "918772575e48e81e455818229bf719d4ab4181fcbf7f85b68a35620f78d89ced", [:mix], [], "hexpm", "26549a1d6345e2172eb1c233866756ae44a9609bd33ee6f99147ab3fd87fd842"},
+  "plug_crypto": {:hex, :plug_crypto, "2.1.1", "19bda8184399cb24afa10be734f84a16ea0a2bc65054e23a62bb10f06bc89491", [:mix], [], "hexpm", "6470bce6ffe41c8bd497612ffde1a7e4af67f36a15eea5f921af71cf3e11247c"},
   "poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"},
   "protobuf": {:hex, :protobuf, "0.7.1", "7d1b9f7d9ecb32eccd96b0c58572de4d1c09e9e3bc414e4cb15c2dce7013f195", [:mix], [], "hexpm", "6eff7a5287963719521c82e5d5b4583fd1d7cdd89ad129f0ea7d503a50a4d13f"},
   "ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},
-  "remote_ip": {:hex, :remote_ip, "1.1.0", "cb308841595d15df3f9073b7c39243a1dd6ca56e5020295cb012c76fbec50f2d", [:mix], [{:combine, "~> 0.10", [hex: :combine, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "616ffdf66aaad6a72fc546dabf42eed87e2a99e97b09cbd92b10cc180d02ed74"},
+  "remote_ip": {:hex, :remote_ip, "1.2.0", "fb078e12a44414f4cef5a75963c33008fe169b806572ccd17257c208a7bc760f", [:mix], [{:combine, "~> 0.10", [hex: :combine, repo: "hexpm", optional: false]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "2ff91de19c48149ce19ed230a81d377186e4412552a597d6a5137373e5877cb7"},
   "sentry": {:hex, :sentry, "8.0.6", "c8de1bf0523bc120ec37d596c55260901029ecb0994e7075b0973328779ceef7", [:mix], [{:hackney, "~> 1.8", [hex: :hackney, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: true]}, {:plug, "~> 1.6", [hex: :plug, repo: "hexpm", optional: true]}, {:plug_cowboy, "~> 2.3", [hex: :plug_cowboy, repo: "hexpm", optional: true]}], "hexpm", "051a2d0472162f3137787c7c9d6e6e4ef239de9329c8c45b1f1bf1e9379e1883"},
   "ssl_verify_fun": {:hex, :ssl_verify_fun, "1.1.7", "354c321cf377240c7b8716899e182ce4890c5938111a1296add3ec74cf1715df", [:make, :mix, :rebar3], [], "hexpm", "fe4c190e8f37401d30167c8c405eda19469f34577987c76dde613e838bbc67f8"},
-  "telemetry": {:hex, :telemetry, "1.1.0", "a589817034a27eab11144ad24d5c0f9fab1f58173274b1e9bae7074af9cbee51", [:rebar3], [], "hexpm", "b727b2a1f75614774cff2d7565b64d0dfa5bd52ba517f16543e6fc7efcc0df48"},
+  "telemetry": {:hex, :telemetry, "1.3.0", "fedebbae410d715cf8e7062c96a1ef32ec22e764197f70cda73d82778d61e7a2", [:rebar3], [], "hexpm", "7015fc8919dbe63764f4b4b87a95b7c0996bd539e0d499be6ec9d7f3875b79e6"},
   "unicode_util_compat": {:hex, :unicode_util_compat, "0.7.0", "bc84380c9ab48177092f43ac89e4dfa2c6d62b40b8bd132b1059ecc7232f9a78", [:rebar3], [], "hexpm", "25eee6d67df61960cf6a794239566599b09e17e668d3700247bc498638152521"},
   "unsafe": {:hex, :unsafe, "1.0.1", "a27e1874f72ee49312e0a9ec2e0b27924214a05e3ddac90e91727bc76f8613d8", [:mix], [], "hexpm", "6c7729a2d214806450d29766abc2afaa7a2cbecf415be64f36a6691afebb50e5"},
   "uuid": {:hex, :uuid, "1.1.8", "e22fc04499de0de3ed1116b770c7737779f226ceefa0badb3592e64d5cfb4eb9", [:mix], [], "hexpm", "c790593b4c3b601f5dc2378baae7efaf5b3d73c4c6456ba85759905be792f2ac"},

auth/test/auth/cache_test.exs

@@ -1,6 +1,6 @@
 defmodule Auth.CacheTest do
   use ExUnit.Case
-  use Plug.Test
+
   doctest Auth
 
   test "it caches values" do

auth/test/auth/cli_test.exs

@@ -1,6 +1,8 @@
 defmodule Auth.CliTest do
   use ExUnit.Case
-  use Plug.Test
+
+  import Plug.Test
+  import Plug.Conn
 
   describe ".is_call_from_deprecated_cli?" do
     test "no user agent => returns false" do