Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added WIP apikey request guard #185

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions aw-server/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ pub struct AWConfig {
pub port: u16,
#[serde(skip, default = "default_testing")]
pub testing: bool, // This is not written to the config file (serde(skip))
#[serde(default = "default_apikey")]
pub apikey: Option<String>,
#[serde(default = "default_cors")]
pub cors: Vec<String>,
}
Expand All @@ -35,6 +37,7 @@ impl Default for AWConfig {
address: default_address(),
port: default_port(),
testing: default_testing(),
apikey: default_apikey(),
cors: default_cors(),
}
}
Expand Down Expand Up @@ -72,6 +75,10 @@ fn default_testing() -> bool {
is_testing()
}

fn default_apikey() -> Option<String> {
None
}

fn default_port() -> u16 {
if is_testing() {
5666
Expand Down
51 changes: 51 additions & 0 deletions aw-server/src/endpoints/auth.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/// On most systems we do not concern ourselves with local security [1], however on Android
/// apps have their storage isolated yet ActivityWatch happily exposes its API and database
/// through the HTTP API when the server is running. This could be considered a severe
/// security flaw, and fixing it would significantly improve security on Android (as mentioned in [2]).
///
/// Requiring an API key can also be useful in other scenarios where an extra level of security is
/// desired.
///
/// Based on the ApiKey example at [3].
///
/// [1]: https://docs.activitywatch.net/en/latest/security.html#activitywatch-is-only-as-secure-as-your-system
/// [2]: https://forum.activitywatch.net/t/rest-api-supported-with-android-version-of-activity-watch/854/6?u=erikbjare
/// [3]: https://api.rocket.rs/v0.4/rocket/request/trait.FromRequest.html
use rocket::http::Status;
use rocket::request::{self, FromRequest, Request};
use rocket::{Outcome, State};

use crate::config::AWConfig;

struct ApiKey(Option<String>);

#[derive(Debug)]
enum ApiKeyError {
BadCount,
Missing,
Invalid,
}

// TODO: Use guard on endpoints
// TODO: Add tests for protected endpoints (important to ensure security)
impl<'a, 'r> FromRequest<'a, 'r> for ApiKey {
type Error = ApiKeyError;

fn from_request(request: &'a Request<'r>) -> request::Outcome<Self, Self::Error> {
// TODO: How will this key be configured by the user?
let config = request.guard::<State<AWConfig>>().unwrap();
match &config.apikey {
None => Outcome::Success(ApiKey(None)),
Some(apikey) => {
// TODO: How will this header be set in the browser?
let keys: Vec<_> = request.headers().get("x-api-key").collect();
match keys.len() {
0 => Outcome::Failure((Status::BadRequest, ApiKeyError::Missing)),
1 if apikey == keys[0] => Outcome::Success(ApiKey(Some(keys[0].to_string()))),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a very convoluted way of expressing it.
Better to just have a single case for one and have the of statement inside a block.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, just picked it straight from the example.

1 => Outcome::Failure((Status::BadRequest, ApiKeyError::Invalid)),
_ => Outcome::Failure((Status::BadRequest, ApiKeyError::BadCount)),
}
}
}
}
}
11 changes: 11 additions & 0 deletions aw-server/src/endpoints/cors.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
/// Cross-Origin Resource Sharing is a way to specify the permissions websites that do not share the origin have.
///
/// However, it's a check done by the browser when a response has already been received (but before it's accessible by the origin site).
/// As such, it does *not* protect against all kinds of cross-origin attacks. As an example, a GET
/// requests which has side-effects will still have such effects, even though the response was
/// blocked by the browser.
///
/// In many cases, pre-flight requests are made to check CORS before the real request is made.
///
/// For more info, see: https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS
///
use rocket::http::Method;
use rocket_cors::{AllowedHeaders, AllowedOrigins};

Expand Down
1 change: 1 addition & 0 deletions aw-server/src/endpoints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub struct ServerState {

#[macro_use]
mod util;
mod auth;
mod bucket;
mod cors;
mod export;
Expand Down