Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Small changes before moving to kilnauth #5

Merged
merged 4 commits into from Apr 10, 2012

Conversation

Projects
None yet
4 participants
Owner

csilvers commented Apr 10, 2012

Baby steps...

@sophiebits sophiebits commented on the diff Apr 10, 2012

review.py
url_prefix = repo.ui.config('auth', 'kiln.prefix')
if url_prefix is None:
ui.warn("In order to work, in your hgrc please set:\n\n")
ui.warn("[auth]\n")
- ui.warn("kiln.prefix = https://<kilnrepo.kilnhg.com>\n")
- ui.warn("kiln.username = <username>@<domain>.com\n")
+ ui.warn("kiln.prefix = https://<kilnrepo.kilnhg.com> # no trailing /\n")
@sophiebits

sophiebits Apr 10, 2012

Does a trailing slash break things?

@csilvers

csilvers Apr 10, 2012

Owner

Yes, it did in my previous tests. We could maybe fix it but in my experience it's better to just have a strict format and stick to it.

@csilvers

csilvers Apr 10, 2012

Owner

OMG, I just realized that this is showing up as new. I guess I didn't do a pull request for my last set of fixes about a month ago! Based on user feedback of problems.

LGTM

Add the ability to use kilnauth to get an authentication token for re…
…view.

It looks like kilnauth is meant to hook into mercurial.url.open, and
while I got that working, it didn't seem to set credentials the way I
wanted: it looks like the kiln API doesn't automatically take the
token from the fbToken header?

So instead I hack into kilnauth internals a bit in order to extract
the fbToken that it would put in the cookie, and adding it to our url
requests (token=<fbToken>).  Kiln is happy with that.

I also moved the authentication error-checking to the auth routines
where it belongs.

nsfmc commented on d7e1811 Apr 10, 2012

rad

nsfmc commented Apr 10, 2012

lgtm

Explicitly return None when kilnauth_path isn't present.
This is mostly cosmetic, since functions without an explicit return
value implicitly return None in python, but it better shows the
programmer intent.

csilvers added a commit that referenced this pull request Apr 10, 2012

Merge pull request #5 from csilvers/master
Support kilnauth authentication

@csilvers csilvers merged commit 8f60426 into Khan:master Apr 10, 2012

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