[asterisk-dev] [Code Review] 2649: ARI authentication

[Joshua Colp]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/2649/#review9012
-----------------------------------------------------------



/trunk/include/asterisk/http.h
<https://reviewboard.asterisk.org/r/2649/#comment17729>

    Instead of multiple memory allocations you can allocate the object large enough for all the content, like ast_variable does.



/trunk/res/res_stasis_http.c
<https://reviewboard.asterisk.org/r/2649/#comment17725>

    Having this is just silly - you are always going to return whatever ari_config_validate_user returns without doing anything else so just do that.



/trunk/res/res_stasis_http.c
<https://reviewboard.asterisk.org/r/2649/#comment17722>

    Just curious - any reason you went for this instead of strsep? I also think the use of colon for a variable name is meh - it really is the password.



/trunk/res/res_stasis_http.c
<https://reviewboard.asterisk.org/r/2649/#comment17727>

    Use some visine to take the redness out.



/trunk/res/stasis_http/config.c
<https://reviewboard.asterisk.org/r/2649/#comment17730>

    Whilst you are in the area you can allocate this without a lock.



/trunk/res/stasis_http/config.c
<https://reviewboard.asterisk.org/r/2649/#comment17731>

    Ditto - fine without a lock



/trunk/res/stasis_http/config.c
<https://reviewboard.asterisk.org/r/2649/#comment17732>

    Triple - fine without a lock



/trunk/res/stasis_http/config.c
<https://reviewboard.asterisk.org/r/2649/#comment17733>

    I'd elaborate a bit more in this error message - perhaps incorporate the filename?



/trunk/res/stasis_http/config.c
<https://reviewboard.asterisk.org/r/2649/#comment17734>

    Should this be a fatal error for the configuration file? (does this deem it broken) If so then use a prelink callback.


- Joshua Colp


On June 28, 2013, 2:56 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2649/
> -----------------------------------------------------------
> 
> (Updated June 28, 2013, 2:56 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21277
>     https://issues.asterisk.org/jira/browse/ASTERISK-21277
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch adds authentication support to ARI.
> 
> Two authentication methods are supported. The first is HTTP Basic
> authentication, as specified in RFC 2617[1]. The second is by simply
> passing the username and password as an ?api_key query parameter
> (which allows swagger-ui[2] to authenticate more easily).
> 
> ARI usernames and passwords are configured in the stasis_http.conf
> file. The user may be set to `read_only`, which will prohibit the user
> from issuing POST, DELETE, etc. The user's password may be specified
> in either plaintext, or encrypted using the crypt() function.
> 
> Several other notes about the patch.
> 
>  * A few command line commands for seeing ARI config and status were
>    also added.
>  * The configuration parsing grew big enough that I extracted it to
>    its own file.
> 
>  [1]: http://www.ietf.org/rfc/rfc2617.txt
>  [2]: https://github.com/wordnik/swagger-ui
> 
> 
> Diffs
> -----
> 
>   /trunk/configs/stasis_http.conf.sample 393125 
>   /trunk/configure UNKNOWN 
>   /trunk/configure.ac 393125 
>   /trunk/include/asterisk/autoconfig.h.in 393125 
>   /trunk/include/asterisk/http.h 393125 
>   /trunk/include/asterisk/utils.h 393125 
>   /trunk/main/Makefile 393125 
>   /trunk/main/http.c 393125 
>   /trunk/main/utils.c 393125 
>   /trunk/makeopts.in 393125 
>   /trunk/res/Makefile 393125 
>   /trunk/res/res_stasis_http.c 393125 
>   /trunk/res/stasis_http/cli.c PRE-CREATION 
>   /trunk/res/stasis_http/config.c PRE-CREATION 
>   /trunk/res/stasis_http/internal.h PRE-CREATION 
>   /trunk/tests/test_utils.c 393125 
> 
> Diff: https://reviewboard.asterisk.org/r/2649/diff/
> 
> 
> Testing
> -------
> 
> Unit tests for crypt wrapper.
> 
> Testsuite tests for authn testing. See https://reviewboard.asterisk.org/r/2650/
> 
> 
> Thanks,
> 
> David Lee
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130628/5f6ef326/attachment-0001.htm>