feat(IAM-170): adding slack to whoami #9
Conversation
There was a problem hiding this comment.
Can you please run cargo clippy https://github.com/rust-lang/rust-clippy#step-2-install-clippy
and cargo fmt before I take a deeper look?
src/main.rs
Outdated
| )), | ||
| ) | ||
| .service(healthz::healthz_app()) | ||
| }) | ||
| .bind("0.0.0.0:8084")? | ||
| .bind("127.0.0.1:8084")? |
|
|
src/slack/app.rs
Outdated
| slack: &Slack, | ||
| whoami: &WhoAmI, | ||
| secret: &[u8], | ||
| ttl_cache: Arc<RwLock<TtlCache<String, String>>>, |
There was a problem hiding this comment.
That's not used in here. We only use it in the GitHub app to cache usernames.
src/slack/app.rs
Outdated
| ); | ||
| let slack_uri_data: SlackUriData = SlackUriData { | ||
| slack_auth_params, | ||
| direct_message_uri: slack.direct_message_uri.to_string(), |
There was a problem hiding this comment.
that should be a .clone() instead of .to_string() to make it more clear what's happening.
src/slack/app.rs
Outdated
| update_slack( | ||
| format!( | ||
| "{}{}", | ||
| slack_uri_data.direct_message_uri.to_string(), |
There was a problem hiding this comment.
.to_string() is not needed here.
src/slack/app.rs
Outdated
| let slack_token_url = format!( | ||
| "{}{}&code={}", | ||
| TOKEN_URL, | ||
| slack_uri_data.slack_auth_params.to_string(), |
src/slack/app.rs
Outdated
| slack_uri_data: web::Data<SlackUriData>, | ||
| session: Session, | ||
| ) -> Box<dyn Future<Item = HttpResponse, Error = Error>> { | ||
| let code = query.code.clone(); |
There was a problem hiding this comment.
not needed we can use query.code a few lines later directly without copying here.
src/slack/app.rs
Outdated
| auth_url, | ||
| Some(token_url), | ||
| ) | ||
| .add_scope(Scope::new(slack.scope.to_string())) |
There was a problem hiding this comment.
.clone() insteadl of .to_string() to me more explicit.
src/slack/app.rs
Outdated
| HttpResponse::Found() | ||
| .header(http::header::LOCATION, "/e?identityAdded=error") | ||
| .finish() |
There was a problem hiding this comment.
Minor niggle: we could call send_response here
| state: String, | ||
| } | ||
|
|
||
| #[derive(Deserialize, Serialize, Debug)] |
There was a problem hiding this comment.
we can derive Clone here if we delete the impl below.
Adding slack confirmation and handling to whoami application
Jira Story: https://jira.mozilla.com/browse/IAM-170