Artifacts with incomplete scopes are exposed without lock on the UI

RESOLVED FIXED

Status

RESOLVED FIXED
a year ago
8 months ago

People

(Reporter: gerard-majax, Assigned: cynthiapereira)

Tracking

({good-first-bug})

Details

Attachments

(2 attachments)

(Reporter)

Description

a year ago
STR:
 1. Be logged-in with LDAP
 2. On a task where artifacts were misnamed for being public and thus are being private (named "tmp/artifacts/..." instead of "public/...")

Expected:
 I don't have the scope, so there should be a lock next to the artifact as there is when I am not logged in

Actual:
 When not logged in, there is a lock properly shown next to the artifact name. However, once logged in, it disappears. This mislead the user into thinking he has credentials.

Reproduced on https://tools.taskcluster.net/groups/SjAmu9nTTcaCGCurLZLf-w/tasks/L2PvCFjBQlOSRQCdBZilyg/runs/0/artifacts
(Reporter)

Comment 1

a year ago
Created attachment 8891380 [details]
Screenshot-2017-7-28 TaskCluster TensorFlow CPU for OSX 10 12 Taskcluster.png

Lock when not logged in
(Reporter)

Comment 2

a year ago
Created attachment 8891384 [details]
Screenshot-2017-7-28 TaskCluster TensorFlow CPU for OSX 10 12 Taskcluster(1).png

No lock when logged in

Updated

9 months ago
Keywords: good-first-bug
I would like to take this one.
Seems only need to replace getIconFromMime(contentType) in the userSession statement with the icon name.
I think that what we want here is to always show the lock for artifacts that don't begin with `public/`, and to show some other icon suggesting public access (an open lock?) for those that do begin with `public/`.  Regardless of whether the user is logged in or not.

We do not examine user scopes anywhere in the tools site to determine whether clicking a button will work.  I think we should be consistent about that, and not do so here, either.
I mean, we have this conditional used to create a signed URL[1]. No matter if the `runId` is null or not the icon will be get through the `getIconFromMime()` function[2], this is not a problem though. Also we have the same thing for public conditional that works well. However, if the user is not logged and a private signed URL exists, this user will see the lock icon instead the mimetype icon[3]. 

The simple and quickly way to fix it is: replace the `getIconFromMime` here[4] to the lock icon, but if you want to keep showing the mimetype icon according to the file type we will need to show both (file type and lock icon).

So, my question is: which path can I take?

[1] https://github.com/taskcluster/taskcluster-tools/blob/54f5214945d81ab6c3e7f6fab8ae501bd98bb789/src/components/ArtifactList/index.js#L89
[2] https://github.com/taskcluster/taskcluster-tools/blob/54f5214945d81ab6c3e7f6fab8ae501bd98bb789/src/utils.js#L52
[3] https://github.com/taskcluster/taskcluster-tools/blob/54f5214945d81ab6c3e7f6fab8ae501bd98bb789/src/components/ArtifactList/index.js#L108
[4] https://github.com/taskcluster/taskcluster-tools/blob/54f5214945d81ab6c3e7f6fab8ae501bd98bb789/src/components/ArtifactList/index.js#L92
Nice job tracking that down!

I mentioned an open lock for public/.. files, but probably using the mime-type based icon is best.  So:

logged in, public -> mime type icon
logged out, public -> mime type icon
logged in, private -> lock icon
logged out, private -> lock icon

if I'm reading the code right, the patch is one line!  Sometimes figuring out the problem is 99% of the work.

Note that this solution is using the icon to indicate two different things (public/private and mime type). Maybe you or Eli have suggestions of a better way to convey that information -- in which case, I'm happy to hear it!
Assignee: nobody → cynthia

Comment 8

8 months ago
I think the easiest solution would be to show 2 icons: a lock or unlock, and then the icon for mime.
PR updated!

Comment 10

8 months ago
Commit pushed to master at https://github.com/taskcluster/taskcluster-tools

https://github.com/taskcluster/taskcluster-tools/commit/7f618b1cef01e4c56216724fbb2815ff8baf66f1
Bug 1385307 - Show artifact lock icon for logged users (#444)

* Bug 1385307 - Show artifact lock icon for logged users

* Bug 1385307 - Show private artifacts with lock icon

* Bug 1385307 - Show private artifacts with lock icon
Nicely done!
Status: NEW → RESOLVED
Last Resolved: 8 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.