You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by mayya-sharipova <gi...@git.apache.org> on 2015/07/16 14:05:19 UTC
[GitHub] couchdb-couch pull request: 47059 session accept username
GitHub user mayya-sharipova opened a pull request:
https://github.com/apache/couchdb-couch/pull/74
47059 session accept username
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloudant/couchdb-couch 47059_session_accept_username
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/couchdb-couch/pull/74.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #74
----
commit a6aed3c92d0c6239a9f86307e6a8d127d0b5a3b9
Author: Alexander Shorin <kx...@apache.org>
Date: 2015-02-25T18:59:07Z
Move list of system databases to the macro
commit 2d44528adf90623753d1ca02bc57cc64898baad7
Author: Eric Avdey <ei...@eiri.ca>
Date: 2015-05-26T12:59:24Z
Fix `active_size` format conversion in `get_db_info` function
In `active_size` conversion in `couch_db:get_db_info/1` old db reduction's size format assumed to be an integer representing active state, when it also could be a tuple of active and external sizes.
This handeled correctly, for example, in [couch_db_updater.erl](https://github.com/apache/couchdb-couch/blob/master/src/couch_db_updater.erl#L426).
This patch addresses conversion of both possible formats to CouchDB `size_info` record.
This closes issue COUCHDB-2701
commit 6a1f9c7463747572e23ad94c67d7f43278280e15
Author: Eric Avdey <ei...@eiri.ca>
Date: 2015-06-24T14:52:58Z
Rename active_size variable for consistency
commit e4f03c428080edd50a210c1f915c4b8d700b3331
Author: Eric Avdey <ei...@eiri.ca>
Date: 2015-06-24T15:36:10Z
Fix pattern matching order in reduce_sizes
Function `upgrade_sizes` recognises both integer and tuple
data size formats, but pattern matching order in `reduce_sizes`
in `couch_db_updater` do not allow tuple representation
ever reaching it.
commit 4c7ec620eb83e615254e6c7bb45e1f1e9d13a128
Author: Eric Avdey <ei...@eiri.ca>
Date: 2015-06-25T13:32:29Z
Merge pull request #4 from cloudant/47439-fix-active_size-compatibility
Fix `active_size` format conversion in `get_db_info` function
commit 0085ee3db7edc682064d337cb4336c6cbd31b706
Author: Mayya Sharipova <ma...@ca.ibm.com>
Date: 2015-07-16T11:45:47Z
Make _session endpoint accept username in addition to name
Before _session endpoint was only accepting "name" and "password" parameters
This, makes _session endpoint, in addition, to accept "username" and
"password" parameters
BugzId: 47059
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/74#discussion_r36625123
--- Diff: src/couch_httpd_auth.erl ---
@@ -366,6 +366,18 @@ handle_session_req(#httpd{method='DELETE'}=Req, _AuthModule) ->
handle_session_req(Req, _AuthModule) ->
send_method_not_allowed(Req, "GET,HEAD,POST,DELETE").
+
+extract_username(Form) ->
+ CouchFormat = couch_util:get_value("name", Form),
+ try couch_util:get_value("username", Form, CouchFormat) of
--- End diff --
What's the case for having try/catch here? if `couch_util:get_value` will raise badarg due to `Form`, it'll happen on the line above.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-129621129
Nice improvements but the term 'payload' is not right. Say 'request body' and we're good.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-130021531
ah, sure, we should throw 400 Bad Request if you send name and username and they're not the same value.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-121942023
This also needs a JIRA ticket (and no reference to fogbugz). :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-130438870
@mayya-sharipova thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-121940335
Hi! Please rebase your PR against master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-129424336
@kxepal it's rebased now, what do you think? The goal is to accept "username" as an alias for "name" as cloudant's independently developed auth system expects that. I'd rather not maintain a fork of core couchdb for this, so I'd like it to go in.
To the patch itself then, it's the exact same `extract_username` function as in the original codebase that we imported to build couchdb 2.0, so it's a +1 from me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-129425745
@rnewson I'm also +1 for this change, really helpful, except few moment I commented inline.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/74#discussion_r36652415
--- Diff: src/couch_httpd_auth.erl ---
@@ -366,6 +366,19 @@ handle_session_req(#httpd{method='DELETE'}=Req, _AuthModule) ->
handle_session_req(Req, _AuthModule) ->
send_method_not_allowed(Req, "GET,HEAD,POST,DELETE").
+extract_username(Form) ->
+ try couch_util:get_value("name", Form) of
--- End diff --
You miss the question about case of try/catch from previous response (:
There is no such case for this. To prove that let's take a look on L278-288 above.
`Form` expected to be a list, otherwise `couch_util:get_value` will raise `badarg` error. But, it couldn't be non-list.
If Content-Type is `application/x-www-form-urlencoded` then `mochiweb_util:parse_qs/1` called which returns a list.
If Content-Type is `application/json` then we match decoded json against `{Pairs}`. It payload is not a JSON object, we'll have `badmatch` here (btw, that's what is better to gracefully handle instead). Otherwise `Form` will be a list.
For every else Content-Type empty list is assignment.
Any other case I miss when try/catch may be useful here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/74#discussion_r36625136
--- Diff: src/couch_httpd_auth.erl ---
@@ -366,6 +366,18 @@ handle_session_req(#httpd{method='DELETE'}=Req, _AuthModule) ->
handle_session_req(Req, _AuthModule) ->
send_method_not_allowed(Req, "GET,HEAD,POST,DELETE").
+
+extract_username(Form) ->
+ CouchFormat = couch_util:get_value("name", Form),
+ try couch_util:get_value("username", Form, CouchFormat) of
+ undefined ->
+ throw({bad_request, <<"user accounts must have a username">>});
--- End diff --
All around we use 4-spaces indention, not 2.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by rnewson <gi...@git.apache.org>.
Github user rnewson commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-130438132
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by mayya-sharipova <gi...@git.apache.org>.
Github user mayya-sharipova commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-130085650
Hopefully, the last commit addresses all comments :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/couchdb-couch/pull/74
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-130438812
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/74#discussion_r36652825
--- Diff: src/couch_httpd_auth.erl ---
@@ -366,6 +366,19 @@ handle_session_req(#httpd{method='DELETE'}=Req, _AuthModule) ->
handle_session_req(Req, _AuthModule) ->
send_method_not_allowed(Req, "GET,HEAD,POST,DELETE").
+extract_username(Form) ->
+ try couch_util:get_value("name", Form) of
+ CouchFormat ->
+ case couch_util:get_value("username", Form, CouchFormat) of
+ undefined ->
+ throw({bad_request, <<"user accounts must have a username">>});
--- End diff --
I believe, "request payload must contain a username" is more helpful error message, since user don't create any account here, but tries to log in.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/74#discussion_r36652689
--- Diff: src/couch_httpd_auth.erl ---
@@ -366,6 +366,19 @@ handle_session_req(#httpd{method='DELETE'}=Req, _AuthModule) ->
handle_session_req(Req, _AuthModule) ->
send_method_not_allowed(Req, "GET,HEAD,POST,DELETE").
+extract_username(Form) ->
+ try couch_util:get_value("name", Form) of
+ CouchFormat ->
--- End diff --
Not sure why not just `Username`...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by kxepal <gi...@git.apache.org>.
Github user kxepal commented on the pull request:
https://github.com/apache/couchdb-couch/pull/74#issuecomment-129632649
I'm bad with wording (: "request body" is good for me.
Logically, there is one more issue: if both name and username are provided, the latter will take the precedence. Might be good to detect that collision since we read both values in anyway, but I hardly could imagine the case when login form will have them both to POST to let this case hurt.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] couchdb-couch pull request: 47059 session accept username
Posted by mayya-sharipova <gi...@git.apache.org>.
Github user mayya-sharipova commented on a diff in the pull request:
https://github.com/apache/couchdb-couch/pull/74#discussion_r36679606
--- Diff: src/couch_httpd_auth.erl ---
@@ -366,6 +366,19 @@ handle_session_req(#httpd{method='DELETE'}=Req, _AuthModule) ->
handle_session_req(Req, _AuthModule) ->
send_method_not_allowed(Req, "GET,HEAD,POST,DELETE").
+extract_username(Form) ->
+ try couch_util:get_value("name", Form) of
--- End diff --
Thanks for the detailed comments, I tried to address them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---