alexa / alexa-skills-kit-sdk-for-java

The Alexa Skills Kit SDK for Java helps you get a skill up and running quickly, letting you focus on skill logic instead of boilerplate code.
http://developer.amazon.com/ask
Apache License 2.0
815 stars 748 forks source link

List granted permissions #171

Closed PyvesB closed 5 years ago

PyvesB commented 5 years ago

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[ ] Feature request
[x] Documentation issue or request
[ ] Other... Please describe:

Context

I want to determine which permissions the user of my skill has in order to request the missing ones in an AskForPermissionsConsentCard. I tried writing the following simple handler method to print out the list of permissions:

public Optional<Response> handle(HandlerInput input, IntentRequest intentRequest) {
  System.out.println(input.getRequestEnvelope().getSession().getUser().getPermissions());
}

However, even though the user has granted read::alexa:device:all:address, the result is not very informative:

class Permissions {
consentToken: some_token
scopes: null
}

How can I retrieve a list of all granted permissions?

Your Environment

Java Info

Chris-Liao commented 5 years ago

Hi @PyvesB, Request Envelop doesn't contain / maintain a list of Permission, but you can retrieve that from Skill Event when Permission changed, e.g., PermissionChangedRequest or PermissionAcceptedRequest, inside PermissionBody, there is a list of Permission. Hope it helps.

PyvesB commented 5 years ago

Thank you for your response @Chris-Liao. However, I'm unsure how such events would be helpful, as they wouldn't allow to know the state of permissions for a new user request, unless they were used to populate a database mapping from user ID to all permissions that were ever accepted, which is not really practical.

I've got a skill that uses device geolocation and falls back to using the user's address if the device is not geolocation-compatible. The only logic that I managed to get together looks similar to the following (full version in this class):

public Optional<Response> handle(HandlerInput input, IntentRequest intentRequest) {
  if (input.getRequestEnvelope().getContext().getSystem().getDevice().getSupportedInterfaces().getGeolocation() != null) {
    // device is compatible with geolocation
    Permissions permissions = input.getRequestEnvelope().getSession().getUser().getPermissions();
    if (permissions != null && permissions.getScopes() != null && permissions.getScopes().get("alexa::devices:all:geolocation:read").getStatus() == PermissionStatus.GRANTED) {
       // do stuff using geolocation
    } else {
      // request geolocation permissions
    }
  } else {
    // geolocation not available on device, use address instead
    try {
      Address address = deviceAddressServiceClient.getFullAddress(input.getRequestEnvelope().getContext().getSystem().getDevice().getDeviceId());
      // do stuff using address
    } catch (ServiceException e) {
      if (e.getStatusCode() == 403) {
        // request address permissions
      }
    }
  }
}

As you can see, this is not great. To detect whether geolocation permissions are available, there is a lot of method chaining with intermediary null checks. To detect whether address permissions are available in the fallback case, it's even worse, as I have to make a request to the address service to then discover that a 403 is returned and that the user is therefore missing the address permission. Developers must pretty much follow the exceptions as control flow anti-pattern here.

If ever the list of permissions came in the envelope or if there were an easy way of retrieving it, we could significantly clean up the above code and speed up performance by removing requests to the address service that we know in advance would fail.

Am I missing something obvious here? Or is this a shortcoming of the current model returned by Alexa itself and therefore should this feedback be escalated to the appropriate team?

Chris-Liao commented 5 years ago

Hey @PyvesB, you are right that events may not be helpful, since from Skill Permission Accepted Event documentation, it says Skill permissions events for changing or granting permissions always include the most recently accepted permissions in the payload.

Have you tried retrieving Permissions in Context.System.User, which may contain a list of permissions.

For address related permission, most likely you need to call address API and verify status code, as documented here.

PyvesB commented 5 years ago

Have you tried retrieving Permissions in Context.System.User, which may contain a list of permissions.

Retrieving the permissions via input.getRequestEnvelope().getSession().getUser().getPermissions() or via input.getRequestEnvelope().getContext().getSystem().getUser().getPermissions() both lead to the results outlined in my first message, the granted address permission is nowhere to be found. The Permissions object gives an inconsistent and incomplete view of the user's permissions, listing some of the granted permissions but missing others.

For address related permission, most likely you need to call address API and verify status code, as documented here.

As per my previous message, this is what I'm doing and is part of the problem.

Chris-Liao commented 5 years ago

Hey @PyvesB sorry for late response and thanks for patience.

For Geolocation, it's only working with mobile device, like alexa mobile app, you need to grant permission in Alexa app - Skills & Games - Your Skills, and you can see and manage a list of permissions there.

Unfortunately there's no list of granted permission in request envelop. We understand it's not easy to retrieve and verify permission, for the chaining null checks or making service call. This is a really good feature request that I will discuss more with teams, and also suggest you to post your idea to user voice to get more eyes.

pbheemag commented 5 years ago

Hi @PyvesB we’ve communicated your feedback to the team to make sure the work is tracked. Closing this issue. Feel free to reopen for any questions.

PyvesB commented 5 years ago

Thanks for forwarding the feedback!