Skip to content

Conversation

sgrebnov
Copy link
Member

@sgrebnov sgrebnov commented Jan 5, 2015

https://issues.apache.org/jira/browse/CB-8201

Makes it possible to provide custom pluggabble logic to process auth challenge (for example by installing corresponding plugin, sample implementation: https://github.com/msopentech/cordova-plugin-auth-dialog).

Motivation
If you load some resource which requires authentication using android default browser default login dialog appears, but if you try to load the same resource using cordova app then no dialog is shown for user and request is failed w/ 401 Unauthorized. So Currently apps have to provide own JS logic to show web based auth dialog.

It will be great if Cordova Android offers special functionality (in core or via special interface plugin developers can extend) to automatically show native auth dialog. This is extremely useful in corpnet scenarios where all resources requires user to be authenticated.

*
* @param handler The HTTP auth handler.
*/
public void setHttpAuthRequestHandler(HttpAuthRequestHandler handler) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's no direct reason to add this to CordovaActivity, I'd prefer it not added. CordovaActivity has a lot of symbols in it already, and they a little bit just add noise.

@sgrebnov
Copy link
Member Author

Addressed all CR notes, @agrieve could you please take a look

@sgrebnov
Copy link
Member Author

Rebased on top of master branch

* @return Return True if there is a plugin which will resolve this auth challenge, otherwise False
*
*/
public Boolean onReceivedHttpAuthRequest(CordovaWebView view, ICordovaHttpAuthHandler handler, String host, String realm) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless you want this to be tri-state, then "Boolean" should be "boolean". I don't see the need for tristate, so go with "boolean"

@agrieve
Copy link
Contributor

agrieve commented Jan 16, 2015

Just the one nit-pick, then looks good to me! Feel free to just squash and commit it yourself :)

@asfgit asfgit merged commit 11002d4 into apache:master Jan 16, 2015
@sgrebnov
Copy link
Member Author

Done. @agrieve thx for review!

@sgrebnov sgrebnov deleted the CB-8201 branch April 16, 2015 22:05
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