This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2231

narrow: fix for getting the username when running http server
ClosedPublic

Authored by idlsoft on Feb 13 2018, 2:40 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG4224f26c0d35: narrow: fix for getting the username when running http server

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

idlsoft created this revision.Feb 13 2018, 2:40 PM
durin42 accepted this revision.Feb 13 2018, 3:31 PM
This revision is now accepted and ready to land.Feb 13 2018, 3:31 PM
Closed by commit rHG4224f26c0d35: narrow: fix for getting the username when running http server (authored by idlsoft). · Explain WhyFeb 13 2018, 4:07 PM
This revision was automatically updated to reflect the committed changes.
indygreg added a subscriber: indygreg.Feb 14 2018, 1:03 AM
indygreg added inline comments.
hgext/narrow/narrowbundle2.py
331

REMOTE_USER is an HTTP-ism and I think it is a layering violation to look for REMOTE_USER in code that is supposed to be protocol agnostic.

What I'm trying to say is I think we'll need to tweak this ACL code further when it is moved to core.

martinvonz added a subscriber: martinvonz.Feb 15 2018, 5:47 PM
martinvonz added inline comments.
hgext/narrow/narrowbundle2.py
331

But you're okay with this version for now? This commit is currently the bottom-most draft commit in the "committed" repo, so please accept it if you're okay with it. I would have accepted, but I saw this comment and wasn't sure if you thought this was bad enough to not queue it.

indygreg added a comment.Feb 15 2018, 6:32 PM

I think it's fine for now. I accepted this commit.

idlsoft added inline comments.Feb 15 2018, 7:01 PM
hgext/narrow/narrowbundle2.py
331

Ideally I would have liked to have a userid method in repo.ui (or something like that), and have it used by both narrow and acl extension.
But I think this deserves a longer discussion, hence the quick fix.

Revision Contents
Changeset List

PathPackages
Mhgext/narrow/narrowbundle2.py (11 lines)

Diff 5653

hgext/narrow/narrowbundle2.py

repo, common, newmatch, relevant_nodes, ellipsisroots, repo, common, newmatch, relevant_nodes, ellipsisroots,
visitnodes, depth, source, version) visitnodes, depth, source, version)
part = bundler.newpart('changegroup', data=cg) part = bundler.newpart('changegroup', data=cg)
part.addparam('version', version) part.addparam('version', version)
if 'treemanifest' in repo.requirements: if 'treemanifest' in repo.requirements:
part.addparam('treemanifest', '1') part.addparam('treemanifest', '1')
def applyacl_narrow(repo, kwargs): def applyacl_narrow(repo, kwargs):
username = repo.ui.shortuser(repo.ui.username()) ui = repo.ui
user_includes = repo.ui.configlist( username = ui.shortuser(ui.environ.get('REMOTE_USER') or ui.username())
indygregUnsubmitted
Not Done

REMOTE_USER is an HTTP-ism and I think it is a layering violation to look for REMOTE_USER in code that is supposed to be protocol agnostic.

What I'm trying to say is I think we'll need to tweak this ACL code further when it is moved to core.

indygreg: `REMOTE_USER` is an HTTP-ism and I think it is a layering violation to look for `REMOTE_USER`…
martinvonzUnsubmitted
Not Done

But you're okay with this version for now? This commit is currently the bottom-most draft commit in the "committed" repo, so please accept it if you're okay with it. I would have accepted, but I saw this comment and wasn't sure if you thought this was bad enough to not queue it.

martinvonz: But you're okay with this version for now? This commit is currently the bottom-most draft…
idlsoftAuthorUnsubmitted
Not Done

Ideally I would have liked to have a userid method in repo.ui (or something like that), and have it used by both narrow and acl extension.
But I think this deserves a longer discussion, hence the quick fix.

idlsoft: Ideally I would have liked to have a `userid` method in repo.ui (or something like that), and…
user_includes = ui.configlist(
_NARROWACL_SECTION, username + '.includes', _NARROWACL_SECTION, username + '.includes',
repo.ui.configlist(_NARROWACL_SECTION, 'default.includes')) ui.configlist(_NARROWACL_SECTION, 'default.includes'))
user_excludes = repo.ui.configlist( user_excludes = ui.configlist(
_NARROWACL_SECTION, username + '.excludes', _NARROWACL_SECTION, username + '.excludes',
repo.ui.configlist(_NARROWACL_SECTION, 'default.excludes')) ui.configlist(_NARROWACL_SECTION, 'default.excludes'))
if not user_includes: if not user_includes:
raise error.Abort(_("{} configuration for user {} is empty") raise error.Abort(_("{} configuration for user {} is empty")
.format(_NARROWACL_SECTION, username)) .format(_NARROWACL_SECTION, username))
user_includes = [ user_includes = [
'path:.' if p == '*' else 'path:' + p for p in user_includes] 'path:.' if p == '*' else 'path:' + p for p in user_includes]
user_excludes = [ user_excludes = [
'path:.' if p == '*' else 'path:' + p for p in user_excludes] 'path:.' if p == '*' else 'path:' + p for p in user_excludes]