Skip to content

Does loggingServiceV2Client.ALL_SCOPES need cloud-platform scope? #2209

Closed
Closed
Does loggingServiceV2Client.ALL_SCOPES need cloud-platform scope?#2209
Assignees
bcoe
Labels
api: loggingIssues related to the Cloud Logging API.priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Description

@ofrobots
ofrobots
opened on Apr 11, 2017

I am a bit surprised by all the scopes that @google-cloud/logging requests by default:

/**
 * The scopes needed to make gRPC calls to all of the methods defined in
 * this service.
 */
var ALL_SCOPES = [
  'https://www.googleapis.com/auth/cloud-platform',
  'https://www.googleapis.com/auth/cloud-platform.read-only',
  'https://www.googleapis.com/auth/logging.admin',
  'https://www.googleapis.com/auth/logging.read',
  'https://www.googleapis.com/auth/logging.write'
];

Are these all – and specifically cloud-platform – necessary? @lukesneeringer @jmuk.

Activity

  1. ofrobots
    added
    api: loggingIssues related to the Cloud Logging API.
    type: questionRequest for information or clarification. Not an issue.
    on Apr 11, 2017
  2. ofrobots
    added a commit that references this issue on Apr 11, 2017

    logging-winston: default to minimal scopes

    5d0f796

  3. jmuk commented on Apr 11, 2017

    @jmuk
    jmuk
    on Apr 11, 2017
    Contributor

    These are from the service configuration yaml file in https://github.com/googleapis/googleapis/blob/master/google/logging/logging.yaml

    I am not sure about the choice of the scopes, we simply make the union of the all scopes of the rules.

  4. ofrobots commented on Apr 11, 2017

    @ofrobots
    ofrobots
    on Apr 11, 2017
    ContributorAuthor

    I am not the authority on this, but my expectation would be that the following implies that either cloud-platform or logging.admin scopes would work for the specified RPCs.

          canonical_scopes: |-
        https://www.googleapis.com/auth/logging.admin,
        https://www.googleapis.com/auth/cloud-platform

    /cc @munangst: can you confirm?

  5. jmuk commented on Apr 11, 2017

    @jmuk
    jmuk
    on Apr 11, 2017
    Contributor

    To be clear, cloud-platform appears on all three rules, so it needs to be eliminated from all of them if you want. Also please keep in mind that this configuration comes from our internal repository. You need to modify there.

    Reviewing them, it's indeed weird to me too; for example, the second section (mostly for retrieving data) has both cloud-platform.read-only and cloud-platform. I don't know why.

  6. munangst commented on Apr 12, 2017

    @munangst
    munangst
    on Apr 12, 2017

    @ofrobots Yes, you are correct, an OAuth token with any of the entries in the canonical_scopes list will be accepted. You can see this in the documentation for google.api.OAuthRequirements, which is the message used for this config.

    The reason that some API methods accept multiple scopes is to allow different types of OAuth token to access that method. For instance, typically a read-only method like ListLogEntries would accept either cloud-platform.read-only or cloud-platform. A developer who wants the user to authorize access to both read-only and read-write methods would only have to request the cloud-platform scope, while a developer who only wants to authorize read-only methods would request cloud-platform.read-only. Likewise, the logging.* scopes are concentric, so logging.admin allows the admin actions as well as write and read actions.

    (Note also that using scopes for access control is not recommended; Cloud IAM provides much finer-grained control over the set of operations a user or service account is allowed to perform. In most cases it's simplest to have the client request the cloud-platform or cloud-platform.read-only scope and rely on IAM roles to limit what the user can do. However, if you're writing an application for other users, you might want to request a narrower scope or set of scopes, to increase their confidence that the application isn't misusing the privileges the user granted to it.)

  7. ofrobots
    added
    type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.
    and removed
    type: questionRequest for information or clarification. Not an issue.
    on Apr 12, 2017

  8. ofrobots commented on Apr 12, 2017

    @ofrobots
    ofrobots
    on Apr 12, 2017
    ContributorAuthor

    Yes, this would be the library for logging, rather than end user applications – so we should be requesting minimal scopes to avoid misuse of privileges.

    @jmuk, @lukesneeringer converting this to a bug. Does it belong in the google-gax repo?

  9. jmuk commented on Apr 12, 2017

    @jmuk
    jmuk
    on Apr 12, 2017
    Contributor

    Here's the part of the codegen to choose the list of the default scopes (https://github.com/googleapis/toolkit/blob/master/src/main/java/com/google/api/codegen/config/ServiceConfig.java#L48):

    Scopes form a union and the union is used for down-scoping, so adding more scopes that
    are subsets of the others already in the union essentially has no effect.
    We are doing this for implementation simplicity so we don't have to compute which scopes
    are subsets of the others.

    So that's actually an intended behavior for "implementation simplicity" (also this is a common part of the codegen, any languages are affected). However, I don't think this causes problems so far. I mean, I agree that it's better to minimize the coverage of the scopes, but so far I'm not sure how this can be misused.

    I'll open an internal discussion to gather more insights.

  10. lukesneeringer
    added
    priority: p2Moderately-important priority. Fix may not be included in next release.
    on Apr 13, 2017

  11. stephenplusplus commented on May 17, 2017

    @stephenplusplus
    stephenplusplus
    on May 17, 2017
    Contributor

    @jmuk @lukesneeringer @ofrobots what should we do here?

  12. landrito
    added
    status: acknowledged
    on Aug 7, 2017

  13. evaogbe commented on Aug 7, 2017

    @evaogbe
    evaogbe
    on Aug 7, 2017
    Contributor

    @jmuk What was the conclusion from the internal discussion?

  14. jmuk commented on Aug 7, 2017

    @jmuk
    jmuk
    on Aug 7, 2017
    Contributor

    Not much voice, as far as I remember.

    However, considering that this would not lead to any misuses of the scopes and mostly our client use JWT which does not use scopes actually, I think it's okay to close this issue now without doing anything. @anthmgoogle FYI.

  15. evaogbe
    closed this as completedon Aug 7, 2017
  16. JustinBeckwith
    assigned
    evaogbe
    on Feb 1, 2021

  17. evaogbe commented on Feb 2, 2021

    @evaogbe
    evaogbe
    on Feb 2, 2021
    Contributor

    I am no longer on the team

  18. evaogbe
    removed their assignment
    on Feb 2, 2021
  19. JustinBeckwith
    assigned
    bcoe
    on Feb 2, 2021
  20. sofisl
    added 3 commits that reference this issue on Feb 26, 2026

    fix: remove default global trace context propagator (#2209)

    d20168b

    chore(main): release 7.17.1 (#2210)

    d42f3b0

    feat(spanner): add support for Multiplexed Session for Read Only Tran… (

    ca529fa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

api: loggingIssues related to the Cloud Logging API.priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions