-
Notifications
You must be signed in to change notification settings - Fork 46
Camera permission request behavior changes for QR + PIN Auth, Fixes AB#3032520 #2524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
✅ Work item link check complete. Description contains link AB#3032520 to an Azure Boards work item. |
...in/java/com/microsoft/identity/common/internal/providers/oauth2/CameraPermissionRequest.java
Outdated
Show resolved
Hide resolved
...in/java/com/microsoft/identity/common/internal/providers/oauth2/CameraPermissionRequest.java
Outdated
Show resolved
Hide resolved
...va/com/microsoft/identity/common/internal/providers/oauth2/WebViewAuthorizationFragment.java
Show resolved
Hide resolved
|
Didn't the Android OS have a way for the user to configure when they provide permission by saying The work you are doing here, does that take away from user being able to choose |
1 Yes, the OS provide some functionality like this at app level. so, the first time the app needs to access the camera resource it will ask for permissions, then the user can decide if it grant, while using the app , just this time or deny the request. 2 Yes, this change here removes this behavior. why? because this type of QR + PIN flows are for SDM (now). So, the first user can grant the camera permission Note: |
| // The current camera behavior is based on the principle of only prompt once and prompt always. | ||
| // If the OS level permission is not granted, we show the system prompt. | ||
| // If the OS level permission was granted previously, we show the rationale to confirm the consent witth the current user. | ||
| // If the OS level permission was denied previously, we just denny the request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: denny -> deny
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, is there any way to programmatically reset this? (let's say if the user accidentally denies. how do we help them fix that? do we show something to guide them?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is handled by ESTS UX, if the system permission has been permanent denied (Deny, don't ask gain). meaning the user has denied the system permission multiple times.
The only way to restore the permission is through the app info. So, ESTS UX will notify the user that it cannot proceed and that he needs to enable the permission on the app. Notice that the message is generic because the behavior is the same in iOS, Android and Windows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could show a dialog box and send the user to the app info so he can unblock this, but the Android docs disencourage this behavior
https://developer.android.com/training/permissions/requesting
// Explain to the user that the feature is unavailable because
// the feature requires a permission that the user has denied.
// At the same time, respect the user's decision. Don't link to
// system settings in an effort to convince the user to change
// their decision.
There is a new CELA requirement to change the behavior of the camera permission request for QR + PIN Auth.
The current behavior is that once we get the camera permission for the app we never ask again.
The change is to ask every time we need to use the camera.
In addition to this change we move some functionality to the new class CameraPermissionRequest and did some update on the name of the functions.
AB#3032520