You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Gordon Sim <gs...@redhat.com> on 2012/10/12 18:47:15 UTC

Review Request: Abstraction for SASL server role that is not tied to 0-10

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7565/
-----------------------------------------------------------

Review request for qpid, michael goulish, Ted Ross, and mick goulish.


Description
-------

The current broker::SaslAuthenticator mixes 0-10 protocol handshaking with the sasl interactions. This patch adds a cleaner abstraction of the server role in SASL along side that already in place for the client. This is then used in the 1.0 SASL implementation.

(Note this patch assumes and partially includes https://reviews.apache.org/r/7562/ as I could not find an easy way to separate out commits in git into review-board compatible patches).


This addresses bug QPID-4368.
    https://issues.apache.org/jira/browse/QPID-4368


Diffs
-----

  /trunk/qpid/cpp/src/CMakeLists.txt 1397295 
  /trunk/qpid/cpp/src/Makefile.am 1397295 
  /trunk/qpid/cpp/src/qpid/NullSaslServer.h PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/NullSaslServer.cpp PRE-CREATION 
  /trunk/qpid/cpp/src/qpid/Sasl.h 1397295 
  /trunk/qpid/cpp/src/qpid/SaslFactory.h 1397295 
  /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1397295 
  /trunk/qpid/cpp/src/qpid/SaslServer.h PRE-CREATION 

Diff: https://reviews.apache.org/r/7565/diff/


Testing
-------


Thanks,

Gordon Sim


Re: Review Request: Abstraction for SASL server role that is not tied to 0-10

Posted by Gordon Sim <gs...@redhat.com>.

> On Nov. 2, 2012, 6:57 a.m., mick goulish wrote:
> > /trunk/qpid/cpp/src/qpid/NullSaslServer.cpp, line 41
> > <https://reviews.apache.org/r/7565/diff/1/?file=176223#file176223line41>
> >
> >     I feel kind of funny about throwing for an error in an input to the system -- but definitely should log it.    info?  error?

Committed a minor adjustment to log this and return the failure code.


- Gordon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7565/#review13029
-----------------------------------------------------------


On Oct. 12, 2012, 4:47 p.m., Gordon Sim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7565/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2012, 4:47 p.m.)
> 
> 
> Review request for qpid, michael goulish, Ted Ross, and mick goulish.
> 
> 
> Description
> -------
> 
> The current broker::SaslAuthenticator mixes 0-10 protocol handshaking with the sasl interactions. This patch adds a cleaner abstraction of the server role in SASL along side that already in place for the client. This is then used in the 1.0 SASL implementation.
> 
> (Note this patch assumes and partially includes https://reviews.apache.org/r/7562/ as I could not find an easy way to separate out commits in git into review-board compatible patches).
> 
> 
> This addresses bug QPID-4368.
>     https://issues.apache.org/jira/browse/QPID-4368
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/CMakeLists.txt 1397295 
>   /trunk/qpid/cpp/src/Makefile.am 1397295 
>   /trunk/qpid/cpp/src/qpid/NullSaslServer.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/NullSaslServer.cpp PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/Sasl.h 1397295 
>   /trunk/qpid/cpp/src/qpid/SaslFactory.h 1397295 
>   /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1397295 
>   /trunk/qpid/cpp/src/qpid/SaslServer.h PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/7565/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>


Re: Review Request: Abstraction for SASL server role that is not tied to 0-10

Posted by mick goulish <mi...@redhat.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7565/#review13029
-----------------------------------------------------------

Ship it!



/trunk/qpid/cpp/src/qpid/NullSaslServer.cpp
<https://reviews.apache.org/r/7565/#comment28040>

    I feel kind of funny about throwing for an error in an input to the system -- but definitely should log it.    info?  error?


Sorry it took me so long to get to this.
All I can respond to now is overall understandability.
This new code seems hugely more comprehensible / cleaner than what I remember.  
Oh yes, please ship it.

- mick goulish


On Oct. 12, 2012, 4:47 p.m., Gordon Sim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7565/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2012, 4:47 p.m.)
> 
> 
> Review request for qpid, michael goulish, Ted Ross, and mick goulish.
> 
> 
> Description
> -------
> 
> The current broker::SaslAuthenticator mixes 0-10 protocol handshaking with the sasl interactions. This patch adds a cleaner abstraction of the server role in SASL along side that already in place for the client. This is then used in the 1.0 SASL implementation.
> 
> (Note this patch assumes and partially includes https://reviews.apache.org/r/7562/ as I could not find an easy way to separate out commits in git into review-board compatible patches).
> 
> 
> This addresses bug QPID-4368.
>     https://issues.apache.org/jira/browse/QPID-4368
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/CMakeLists.txt 1397295 
>   /trunk/qpid/cpp/src/Makefile.am 1397295 
>   /trunk/qpid/cpp/src/qpid/NullSaslServer.h PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/NullSaslServer.cpp PRE-CREATION 
>   /trunk/qpid/cpp/src/qpid/Sasl.h 1397295 
>   /trunk/qpid/cpp/src/qpid/SaslFactory.h 1397295 
>   /trunk/qpid/cpp/src/qpid/SaslFactory.cpp 1397295 
>   /trunk/qpid/cpp/src/qpid/SaslServer.h PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/7565/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gordon Sim
> 
>