Add FLAG_ACTIVITY_NEW_TASK when context is not an Activity to prevent crashing by SubSide · Pull Request #765 · openid/AppAuth-Android · GitHub
Skip to content

Add FLAG_ACTIVITY_NEW_TASK when context is not an Activity to prevent crashing#765

Closed
SubSide wants to merge 1 commit into
openid:masterfrom
NPOStart-ScaleUp:master
Closed

Add FLAG_ACTIVITY_NEW_TASK when context is not an Activity to prevent crashing#765
SubSide wants to merge 1 commit into
openid:masterfrom
NPOStart-ScaleUp:master

Conversation

@SubSide

@SubSide SubSide commented Nov 19, 2021

Copy link
Copy Markdown
Contributor

Checklist

  • I read the Contribution Guidelines
  • I signed the CLA and WG Agreements (I signed them, but I'm not sure where this link can be found)
  • I ran, updated and added unit tests as necessary.
  • I verified the contribution matches existing coding style.
  • I updated the documentation if necessary.

Motivation and Context

In our project we work with clean architecture. Where we access the auth service we don't have access to an Activity context but just the ApplicationContext. When we do perform requests with the Application Context, AppAuth will crash as the intent created needs the FLAG_ACTIVITY_NEW_TASK flag.

Description

The change is minor. If the context is not an Activity we add the FLAG_ACTIVITY_NEW_TASK flag. We try to be as conservative as possible with this by unwrapping the Context if it's a ContextWrapper and checking the root. (As startIntent will be delegated upwards by ContextWrapper to the parent Context as well).

@SubSide

SubSide commented Dec 2, 2021

Copy link
Copy Markdown
Contributor Author

@agologan

agologan commented Dec 7, 2021

Copy link
Copy Markdown
Collaborator

Approved and merged your changes here: 66d41d2 with a small caveat.

While the comment and the explanation correctly explains the caller needs to be an activity or specify a task id or create a new task, the check was done on the context used in the intent creation not the actual caller. This works in this case because the context used is the same but it's not really correct. As such, I moved your code to performAuthManagementRequest:.

Let me know if this is what you expected and if you have further concerns.

@SubSide

SubSide commented Dec 8, 2021

Copy link
Copy Markdown
Contributor Author

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants