Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(859)

Issue 92560043: Validate usernames

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 4 months ago by waigani
Modified:
6 years, 4 months ago
Reviewers:
menn0, mp+220535
Visibility:
Public.

Description

Validate usernames Move username regex validation into names.IsUser. Update codebase to ensure usernames are validated where need. There are a few comments which are questions for the reviewer. I will remove them on LGTM. https://code.launchpad.net/~waigani/juju-core/use_vailduser_regex/+merge/220535 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 19

Patch Set 2 : Validate usernames #

Unified diffs Side-by-side diffs Delta from patch set Stats (+63 lines, -46 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M charm/config_test.go View 1 chunk +1 line, -1 line 0 comments Download
M charm/url.go View 2 chunks +3 lines, -2 lines 0 comments Download
M charm/url_test.go View 1 1 chunk +2 lines, -12 lines 0 comments Download
M names/user.go View 1 chunk +4 lines, -3 lines 0 comments Download
M names/user_test.go View 1 2 chunks +42 lines, -5 lines 0 comments Download
M provider/azure/environ.go View 1 chunk +1 line, -0 lines 0 comments Download
M provider/manual/provider.go View 1 chunk +1 line, -1 line 0 comments Download
M state/api/usermanager/client.go View 1 2 chunks +5 lines, -0 lines 0 comments Download
M state/user.go View 3 chunks +1 line, -4 lines 0 comments Download
M state/user_test.go View 1 1 chunk +1 line, -18 lines 0 comments Download

Messages

Total messages: 4
waigani
Please take a look.
6 years, 4 months ago (2014-05-21 22:08:40 UTC) #1
menn0
Just a few things. https://codereview.appspot.com/92560043/diff/1/agent/mongo/upgrade.go File agent/mongo/upgrade.go (right): https://codereview.appspot.com/92560043/diff/1/agent/mongo/upgrade.go#newcode61 agent/mongo/upgrade.go:61: } Is this really necessary? ...
6 years, 4 months ago (2014-05-21 23:06:16 UTC) #2
waigani
Please take a look. https://codereview.appspot.com/92560043/diff/1/agent/mongo/upgrade.go File agent/mongo/upgrade.go (right): https://codereview.appspot.com/92560043/diff/1/agent/mongo/upgrade.go#newcode61 agent/mongo/upgrade.go:61: } On 2014/05/21 23:06:15, menn0 ...
6 years, 4 months ago (2014-05-22 00:59:14 UTC) #3
menn0
6 years, 4 months ago (2014-05-22 01:13:44 UTC) #4
Nice. LGTM

https://codereview.appspot.com/92560043/diff/1/provider/joyent/config.go
File provider/joyent/config.go (right):

https://codereview.appspot.com/92560043/diff/1/provider/joyent/config.go#newc...
provider/joyent/config.go:61: // validate them?
On 2014/05/22 00:59:13, waigani wrote:
> On 2014/05/21 23:06:15, menn0 wrote:
> > These loook like joyent specific account details. We should apply the Juju
> user
> > name policy to these as there's no guarantee that their policy matches ours.
> 
> We should NOT apply, right? How do provider specific user accounts relate to
> juju users, if at all?

Sorry, missed a word. Definitely NOT :)

https://codereview.appspot.com/92560043/diff/1/provider/openstack/config.go
File provider/openstack/config.go (right):

https://codereview.appspot.com/92560043/diff/1/provider/openstack/config.go#n...
provider/openstack/config.go:148: // Do we want to validate username here?
On 2014/05/22 00:59:13, waigani wrote:
> On 2014/05/21 23:06:15, menn0 wrote:
> > Same as the joyent case. We shouldn't apply Juju's user name policy to
> openstack
> > usernames.
> 
> Same question as joyent case. How do provider users relate to juju users? A
> provider will expect users that conform to their policies when authenticating
> and authorising use of resources within it's environment.
> 
> Turning the question on its head, does Juju's user name policy need to conform
> to the provider's policy? Will juju users be proxies for provider users?

The provider account details and juju usernames live in different worlds.
There's no real relationship between them.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b