coveralls-public icon indicating copy to clipboard operation
coveralls-public copied to clipboard

Coveralls shouldn't need `commit:status` scope just to view reports

Open danielbeardsley opened this issue 3 years ago • 7 comments

Problem

Coveralls shouldn't need the commit:status permission just to view reports. I understand that scope is needed by the service itself to write the commit statuses, but it seems incorrect to require that permission from every viewer of the reports.

image

Expectation

I expect coveralls to use Oauth merely to get an email address or to determine Org or Team membership when users are only viewing reports.

danielbeardsley avatar May 11 '21 23:05 danielbeardsley

Hi @danielbeardsley.

So to clarify, your use case is that you have some collaborators who should be able to login to coveralls to review coverage reports, but should not be able to post status updates?

Right now we do assume each user is a developer on the project and what coveralls access gives them is the ability to submit coverage reports (from CI to coveralls). Given that any user of coveralls can do that, any user should be able to have those coverage results generate and submit a status check back to the repo.

Up to now, the approach for providing a read-only view of coverage stats is our Notifications feature, at [Your Coveralls Repo] > Settings > Notifications. Whether by Slack, or email, or generic webhook, those notifications can be used to provide non-developers with project coverage updates.

afinetooth avatar May 14 '21 20:05 afinetooth

Not exactly. The tool I understand needs to be able to update commit statuses upon receiving a coverage log. But it seems like a lot of overreach to say that every viewer of the coverage log needs to to allow coveralls to update commit statuses for all of their own private repos. No other CI tool I've seen (CodeCov, Travis, ...) requires that all viewers of the logs need to give the tool write-access to the statuses of all their repos.

danielbeardsley avatar May 14 '21 21:05 danielbeardsley

I understand. We have considered creating different user levels, for instance "Billing Manager", but for the history of the project we've always considered every user a developer.

I suppose we could update scopes when a user adds a repo to coveralls and funnel all status updates through that user.

If you're willing to elaborate on the permissions model you have in mind, with an approach to making status updates, I will certainly add it for consideration to the card in our backlog re: multiple access levels.

Another approach (planned feature) is to switch to using a Github app (vs OAuth app) to make both status updates and PR comments.

afinetooth avatar May 14 '21 22:05 afinetooth

Thanks for listening. Every user on my team is a developer, but currently, to use your tool, we all have to give the tool commit status write access to all our individual / personal / private / public repos. I think the tool ought to only need to write to one repo as one user and thus only need one key. Most other CI platforms work this way.

We have a CI server that runs a coverage repo and uploads it. That action is the only time anything is "writing" to Coveralls. The developers mostly then read the reports that have been uploaded and reported as statuses.

danielbeardsley avatar May 14 '21 22:05 danielbeardsley

Thanks, @danielbeardsley. I have added your description to the appropriate card(s) in our backlog.

afinetooth avatar May 19 '21 18:05 afinetooth

Just to second @danielbeardsley 's request here, my biggest concern is that I would need to grant access to all my repos that currently aren't using coveralls at all. Maybe if there's some particular feature that requires viewers to grant that scope, allow me to view without it and then ask if I want to grant the scope when I try to use the feature. As it is, I was linked to a report from another team member but couldn't view it since I didn't want to grant access to every repo.

sterlinghirsh avatar Sep 08 '21 19:09 sterlinghirsh

Hi @sterlinghirsh. Understood. I have added your comments to the existing card for this enhancement request. Thanks for your input.

afinetooth avatar Sep 08 '21 22:09 afinetooth