Mask shortSha field in CiJob if user is not allowed to see job
The following discussion from !129831 (merged) should be addressed:
-
@vshushlin started a discussion: (+2 comments) Question/suggestion: shall
short_sha
be private?I remember us being quite strict on the "everything about the code should be private".
WDYT?
updated_at runner].freeze
Since the CiJob.shortSha
field is non-nullable, we can't return false from Types::Ci::JobBaseField#authorized?
without triggering an exception that will remove the job from the result set. An alternative is to leverage the unauthorized_field hook, but that requires changes to GitlabSchema
, instead of being localized to Types::Ci::JobType
or Types::Ci::JobBaseField
. We could opt to make it generic in the sense that any non-nullable string field is returned as an empty string whenever it is not authorized:
diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index 0c7195c5be37..0605b7fa7c06 100644
--- a/app/graphql/gitlab_schema.rb
+++ b/app/graphql/gitlab_schema.rb
@@ -101,6 +101,15 @@ def find_by_gid(gid)
end
end
+ def unauthorized_field(error)
+ if error.field.type.is_a?(GraphQL::Schema::NonNull) && error.field.type.of_type == GraphQL::Types::String
+ # Return an empty string when a non-nullable string field is not authorized
+ return ''
+ end
+
+ nil
+ end
+
# Parse a string to a GlobalID, raising ArgumentError if there are problems
# with it.
#
diff --git a/app/graphql/types/ci/job_base_field.rb b/app/graphql/types/ci/job_base_field.rb
index cc63a53bfc8c..1bb6b51bfdee 100644
--- a/app/graphql/types/ci/job_base_field.rb
+++ b/app/graphql/types/ci/job_base_field.rb
@@ -7,7 +7,7 @@ module Ci
# rubocop: disable Graphql/AuthorizeTypes
class JobBaseField < ::Types::BaseField
PUBLIC_FIELDS = %i[allow_failure duration id kind status created_at finished_at queued_at queued_duration
- updated_at runner short_sha].freeze
+ updated_at runner].freeze
def authorized?(object, args, ctx)
current_user = ctx[:current_user]
Another more controlable option (suggested by @acroitor
) is the following:
field :some_field, if_unauthorized: "No permission"
def unauthorized_field(error)
return error.field.if_unauthorized if error.field.if_unauthorized
raise error
end