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

Tapir based secuirty #794

Merged
merged 7 commits into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions .scalafmt.conf
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
version=3.3.3
maxColumn = 140
runner.dialect = scala213
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ class Http() extends Tapir with TapirJsonCirce with TapirSchemas with StrictLogg
/** Base endpoint description for secured endpoints. Specifies that errors are always returned as JSON values corresponding to the
* [[Error_OUT]] class, and that authentication is read from the `Authorization: Bearer` header.
*/
val secureEndpoint: Endpoint[Unit, Id, (StatusCode, Error_OUT), Unit, Any] =
baseEndpoint.in(auth.bearer[String]().map(_.asInstanceOf[Id])(identity))
val secureEndpoint: Endpoint[Id, Unit, (StatusCode, Error_OUT), Unit, Any] =
baseEndpoint.securityIn(auth.bearer[String]().map(_.asInstanceOf[Id])(identity))

//

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package com.softwaremill.bootzooka.security

import java.security.SecureRandom
import java.time.Instant
import cats.data.OptionT
import cats.effect.IO
import com.softwaremill.bootzooka._
Expand All @@ -11,6 +9,8 @@ import com.softwaremill.bootzooka.util._
import com.softwaremill.tagging._
import com.typesafe.scalalogging.StrictLogging

import java.security.SecureRandom
import java.time.Instant
import scala.concurrent.duration._

class Auth[T](
Expand Down Expand Up @@ -42,6 +42,11 @@ class Auth[T](
}
}

def applyEither(id: Id): IO[Either[Fail.Unauthorized, Id @@ User]] =
apply(id)
.map(i => Right(i))
.handleError(_ => Left(Fail.Unauthorized("Unauthorized")))

private def verifyValid(token: T): IO[Option[Unit]] = {
clock.now[IO]().flatMap { time =>
if (time.isAfter(authTokenOps.validUntil(token))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,36 +44,36 @@ class UserApi(http: Http, auth: Auth[ApiKey], userService: UserService, xa: Tran
}

private val changePasswordEndpoint = secureEndpoint.post
.in(UserPath / "changepassword")
.securityIn(UserPath / "changepassword")
.in(jsonBody[ChangePassword_IN])
.out(jsonBody[ChangePassword_OUT])
.serverLogic { case (authData, data) =>
.serverSecurityLogic(authData => auth.applyEither(authData).map(_.left.map(exceptionToErrorOut)))
Copy link
Member

Choose a reason for hiding this comment

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

can't we use .toOut here as for server logic?

Copy link
Contributor Author

@Pask423 Pask423 Jan 31, 2022

Choose a reason for hiding this comment

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

Not if I understand it correctly here we just mapping left while .toOut perform mapping on both left and right, in fact .toOut maps whole IO[] we just need to map the left of Either

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't understand correctly fixed

.serverLogic(id => data =>
(for {
userId <- auth(authData)
_ <- userService.changePassword(userId, data.currentPassword, data.newPassword).transact(xa)
_ <- userService.changePassword(id, data.currentPassword, data.newPassword).transact(xa)
} yield ChangePassword_OUT()).toOut
}
)

private val getUserEndpoint = secureEndpoint.get
.in(UserPath)
Copy link
Member

Choose a reason for hiding this comment

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

if changePasswordEndpoint uses .securityIn for the path, shouldn't we use this here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I am not sure if changePasswordEndpoint require path in .securityIn , which way is better in your opinion ?

Copy link
Member

Choose a reason for hiding this comment

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

yeah you're right it's fine to keep them all in the regular inputs. The path matching is done on both anyway

.out(jsonBody[GetUser_OUT])
.serverLogic { authData =>
.serverSecurityLogic(authData => auth.applyEither(authData).map(_.left.map(exceptionToErrorOut)))
.serverLogic(id => (_: Unit) =>
(for {
userId <- auth(authData)
user <- userService.findById(userId).transact(xa)
user <- userService.findById(id).transact(xa)
} yield GetUser_OUT(user.login, user.emailLowerCased, user.createdOn)).toOut
}
)

private val updateUserEndpoint = secureEndpoint.post
.in(UserPath)
.in(jsonBody[UpdateUser_IN])
.out(jsonBody[UpdateUser_OUT])
.serverLogic { case (authData, data) =>
.serverSecurityLogic(authData => auth.applyEither(authData).map(_.left.map(exceptionToErrorOut)))
.serverLogic(id => data =>
(for {
userId <- auth(authData)
_ <- userService.changeUser(userId, data.login, data.email).transact(xa)
_ <- userService.changeUser(id, data.login, data.email).transact(xa)
} yield UpdateUser_OUT()).toOut
}
)

val endpoints: ServerEndpoints =
NonEmptyList
Expand Down
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import scala.sys.process.Process
import complete.DefaultParsers._

val doobieVersion = "1.0.0-RC2"
val http4sVersion = "0.23.8"
val http4sVersion = "0.23.9"
val circeVersion = "0.14.1"
val tsecVersion = "0.4.0"
val sttpVersion = "3.4.1"
Expand Down