Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Check hostname on basic auth #2623

Merged
merged 2 commits into from
Jul 22, 2016
Merged

Check hostname on basic auth #2623

merged 2 commits into from
Jul 22, 2016

Conversation

darkdh
Copy link
Member

@darkdh darkdh commented Jul 21, 2016

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Ran git rebase -i to squash commits if needed.

fix #1997

@darkdh
Copy link
Member Author

darkdh commented Jul 21, 2016

@darkdh
Copy link
Member Author

darkdh commented Jul 21, 2016

For https://portal.adp.com/wps/employee/employee.jsp, maybe we should handle request.url which is https://agateway.adp.com/siteminderagent/nocert/1469080019/smgetcred.scc?TYPE=16777217&REALM=-SM-Portal%20Access%20[01%3a46%3a59%3a140454020514026]&SMAUTHREASON=0&METHOD=GET&SMAGENTNAME=-SM-WUbh%2fKU6%2fYJkT8Rq%2f28f9RmfcrwuLShn6MigyjJ%2fEMSZUGriXI1cua%2f6JFOBq%2fGn&TARGET=-SM-https%3a%2f%2fportal%2eadp%2ecom%2fwps%2femployee%2femployee%2ejsp before sending LOGIN_REQUIRED

@bbondy
Copy link
Member

bbondy commented Jul 21, 2016

@diracdeltas pls review but seems good to me.

const frames = self.props.windowState.get('frames').filter((frame) => frame.get('location') === detail.url)
const frames = self.props.windowState.get('frames')
.filter((frame) => frame.get('location') === detail.url ||
urlParse(frame.get('location')).hostname === urlParse(detail.url).hostname)
Copy link
Member

Choose a reason for hiding this comment

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

seems better to check by origin (scheme + hostname + port) instead of just hostname. see getOrigin in js/state/siteUtil.js.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your advice, @diracdeltas.

@bbondy
Copy link
Member

bbondy commented Jul 22, 2016

++ thanks

@bbondy bbondy merged commit d345066 into brave:master Jul 22, 2016
@luixxiul luixxiul modified the milestones: 0.11.3dev, 0.11.2dev Jul 22, 2016
@darkdh darkdh deleted the 1997 branch August 4, 2016 15:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Basic auth doesn't seem to work on this page
4 participants