Implement Basic Auth for API and Owner API #1566
Conversation
quentinlesceller
changed the title
quentinlesceller
changed the title
|
👍 Looks like a good start at wallet auth (and as a bonus doesn't look like it conflicts with T4 wallet changes) |
|
Great, I'm waiting for @hashmap per router middleware instead of per route middleware improvement to reduce the number of line of codes. |
| @@ -878,7 +885,7 @@ pub fn build_router( | |||
| }; | |||
|
|
|||
| let mut router = Router::new(); | |||
| // example how we can use midlleware | |||
|
|
|||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/bin/cmd/client.rs
Outdated
| @@ -117,7 +127,16 @@ pub fn unban_peer(config: &ServerConfig, peer_addr: &SocketAddr) { | |||
| pub fn list_connected_peers(config: &ServerConfig) { | |||
| let mut e = term::stdout().unwrap(); | |||
| let url = format!("http://{}/v1/peers/connected", config.api_http_addr); | |||
| match api::client::get::<Vec<p2p::PeerInfo>>(url.as_str()).map_err(|e| Error::API(e)) { | |||
| let peers_info: Result<Vec<p2p::PeerInfo>, api::Error>; | |||
| if config.api_basic_auth.is_some() && config.api_basic_auth.unwrap() { | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
wallet/src/types.rs
Outdated
| @@ -52,6 +57,8 @@ impl Default for WalletConfig { | |||
| chain_type: Some(ChainTypes::Testnet3), | |||
| api_listen_interface: "127.0.0.1".to_string(), | |||
| api_listen_port: 13415, | |||
| owner_api_basic_auth: Some(true), | |||
| owner_api_secret: thread_rng().sample_iter(&Alphanumeric).take(20).collect(), | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
api/src/client.rs
Outdated
|
|
||
| pub type ClientResponseFuture<T> = Box<Future<Item = T, Error = Error> + Send>; | ||
|
|
||
| /// Helper function to easily issue a HTTP GET request against a given URL that | ||
| /// returns a JSON object. Handles request building, JSON deserialization and | ||
| /// response code checking. | ||
| pub fn get<'a, T>(url: &'a str) -> Result<T, Error> | ||
| pub fn get<'a, T>(url: &'a str, api_basic_auth: bool, api_secret: &'a str) -> Result<T, Error> |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
api/src/client.rs
Outdated
| .map_err(|e| { | ||
| ErrorKind::RequestError(format!("Bad request {} {}: {}", method, url, e)).into() | ||
| }) | ||
| if api_basic_auth { |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
servers/src/common/types.rs
Outdated
| @@ -113,6 +114,12 @@ pub struct ServerConfig { | |||
| /// Network address for the Rest API HTTP server. | |||
| pub api_http_addr: String, | |||
|
|
|||
| /// Whether to enable basic access authentication on API | |||
| pub api_basic_auth: Option<bool>, | |||
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
servers/src/common/types.rs
Outdated
| pub api_basic_auth: Option<bool>, | ||
|
|
||
| /// Secret for basic auth on Rest API HTTP server. | ||
| pub api_secret: String, |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
👍 |
|
Can't do a code review atm but the changes based on your comment sgtm
(sounds good to me)
…On Wed, Sep 26, 2018, 18:31 Quentin Le Sceller ***@***.***> wrote:
@hashmap <https://github.com/hashmap> @Kargakis
<https://github.com/kargakis> can you take a look. Now grin will generate
a .api_secret file under grin directory. You can comment the line to
disable basic auth in server and wallet configs. Basic Auth is enabled by
default. Both APIs use the same secret.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1566 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADuFf_Df5l4PrI5XF4rY66F_9LCnojp_ks5ue6vSgaJpZM4WyerD>
.
|
Fixes #1290.
@hashmap many thanks for the middleware implementation.
Grin wallet generates an
api_secretandowner_api_secretthat can be used to query both API with Basic Auth.This basic authentication can be disabled/enabled in the
grin-server.toml/grin-wallet.tomlbut is enabled by default.Feedbacks are welcome.
I'll add a bit of documentation later today.