Skip to content

Commit

Permalink
Issue open-horizon#629: Stopped user GET routes from returning hashed…
Browse files Browse the repository at this point in the history 
… passwords; they now only return '********' for passwords

Signed-off-by: Ethan Weaver <[email protected]>
  • Loading branch information
@ewee33
ewee33 committed Jun 27, 2022
1 parent be7f87e commit 60c84b1
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 39 deletions.
4 changes: 2 additions & 2 deletions src/main/scala/com/horizon/exchangeapi/UsersRoutes.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ trait UsersRoutes extends JacksonSupport with AuthenticationSupport {
val query = if (ident.isHubAdmin && !ident.isSuperUser) UsersTQ.getAllAdmins(orgid) else UsersTQ.getAllUsers(orgid)
db.run(query.result).map({ list =>
logger.debug(s"GET /orgs/$orgid/users result size: ${list.size}")
val users: Map[String, User] = list.map(e => e.username -> User(if (ident.isSuperUser || ident.isHubAdmin) e.hashedPw else StrConstants.hiddenPw, e.admin, e.hubAdmin, e.email, e.lastUpdated, e.updatedBy)).toMap
val users: Map[String, User] = list.map(e => e.username -> User(StrConstants.hiddenPw, e.admin, e.hubAdmin, e.email, e.lastUpdated, e.updatedBy)).toMap
val code: StatusCode with Serializable = if (users.nonEmpty) StatusCodes.OK else StatusCodes.NotFound
(code, GetUsersResponse(users, 0))
})
Expand Down Expand Up @@ -228,7 +228,7 @@ trait UsersRoutes extends JacksonSupport with AuthenticationSupport {
val query = if (ident.isHubAdmin && !ident.isSuperUser) UsersTQ.getUserIfAdmin(compositeId) else UsersTQ.getUser(compositeId)
db.run(query.result).map({ list =>
logger.debug(s"GET /orgs/$orgid/users/$realUsername result size: ${list.size}")
val users: Map[String, User] = list.map(e => e.username -> User(if (ident.isSuperUser || ident.isHubAdmin) e.hashedPw else StrConstants.hiddenPw, e.admin, e.hubAdmin, e.email, e.lastUpdated, e.updatedBy)).toMap
val users: Map[String, User] = list.map(e => e.username -> User(StrConstants.hiddenPw, e.admin, e.hubAdmin, e.email, e.lastUpdated, e.updatedBy)).toMap
val code: StatusCode with Serializable = if (users.nonEmpty) StatusCodes.OK else StatusCodes.NotFound
(code, GetUsersResponse(users, 0))
})
Expand Down
33 changes: 12 additions & 21 deletions src/test/scala/com/horizon/exchangeapi/route/user/TestGetUserRoute.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,6 @@ class TestGetUserRoute extends AnyFunSuite with BeforeAndAfterAll {
}

def assertUsersEqual(user1: User, user2: UserRow): Unit = {
assert(user1.password === user2.hashedPw)
assert(user1.admin === user2.admin)
assert(user1.hubAdmin === user2.hubAdmin)
assert(user1.email === user2.email)
assert(user1.lastUpdated === user2.lastUpdated)
assert(user1.updatedBy === user2.updatedBy)
}

def assertUsersEqualNoPass(user1: User, user2: UserRow): Unit = {
assert(user1.password === StrConstants.hiddenPw)
assert(user1.admin === user2.admin)
assert(user1.hubAdmin === user2.hubAdmin)
Expand Down Expand Up @@ -202,7 +193,7 @@ class TestGetUserRoute extends AnyFunSuite with BeforeAndAfterAll {
assert(responseBody.users.isEmpty)
}

test("GET /orgs/root" + ROUTE + "root -- as root -- 200 success, returns self w/ hashed password") {
test("GET /orgs/root" + ROUTE + "root -- as root -- 200 success, returns self") {
val response: HttpResponse[String] = Http(URL + "root" + ROUTE + "root").headers(ACCEPT).headers(ROOTAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
Expand All @@ -212,7 +203,7 @@ class TestGetUserRoute extends AnyFunSuite with BeforeAndAfterAll {
assert(responseBody.users.contains("root/root"))
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + TESTUSERS(2).username.split("/")(1) + " -- as root -- 200 success, returns user w/ hashed passwords") {
test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + TESTUSERS(2).username.split("/")(1) + " -- as root -- 200 success, returns user") {
val response: HttpResponse[String] = Http(URL + TESTORGS(0).orgId + ROUTE + TESTUSERS(2).username.split("/")(1)).headers(ACCEPT).headers(ROOTAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
Expand All @@ -223,7 +214,7 @@ class TestGetUserRoute extends AnyFunSuite with BeforeAndAfterAll {
assertUsersEqual(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + TESTUSERS(1).username.split("/")(1) + " -- as hub admin -- 200 success, returns admin user w/ hashed passwords") {
test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + TESTUSERS(1).username.split("/")(1) + " -- as hub admin -- 200 success, returns admin user") {
val response: HttpResponse[String] = Http(URL + TESTORGS(0).orgId + ROUTE + TESTUSERS(1).username.split("/")(1)).headers(ACCEPT).headers(HUBADMINAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
Expand All @@ -234,7 +225,7 @@ class TestGetUserRoute extends AnyFunSuite with BeforeAndAfterAll {
assertUsersEqual(responseBody.users(TESTUSERS(1).username), TESTUSERS(1))
}

test("GET /orgs/root" + ROUTE + "root -- as hub admin -- 200 success, returns root user w/ hashed password") {
test("GET /orgs/root" + ROUTE + "root -- as hub admin -- 200 success, returns root user") {
val response: HttpResponse[String] = Http(URL + "root" + ROUTE + "root").headers(ACCEPT).headers(HUBADMINAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
Expand All @@ -244,26 +235,26 @@ class TestGetUserRoute extends AnyFunSuite with BeforeAndAfterAll {
assert(responseBody.users.contains("root/root"))
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + TESTUSERS(2).username.split("/")(1) + " -- as org admin -- 200 success, returns user w/o password") {
test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + TESTUSERS(2).username.split("/")(1) + " -- as org admin -- 200 success, returns user") {
val response: HttpResponse[String] = Http(URL + TESTORGS(0).orgId + ROUTE + TESTUSERS(2).username.split("/")(1)).headers(ACCEPT).headers(ORG1ADMINAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
assert(response.code === HttpCode.OK.intValue)
val responseBody: GetUsersResponse = JsonMethods.parse(response.body).extract[GetUsersResponse]
assert(responseBody.users.size === 1)
assert(responseBody.users.contains(TESTUSERS(2).username))
assertUsersEqualNoPass(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
assertUsersEqual(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + TESTUSERS(2).username.split("/")(1) + " -- as user -- 200 success, retuns self w/o password") {
test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + TESTUSERS(2).username.split("/")(1) + " -- as user -- 200 success, retuns self") {
val response: HttpResponse[String] = Http(URL + TESTORGS(0).orgId + ROUTE + TESTUSERS(2).username.split("/")(1)).headers(ACCEPT).headers(ORG1USERAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
assert(response.code === HttpCode.OK.intValue)
val responseBody: GetUsersResponse = JsonMethods.parse(response.body).extract[GetUsersResponse]
assert(responseBody.users.size === 1)
assert(responseBody.users.contains(TESTUSERS(2).username))
assertUsersEqualNoPass(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
assertUsersEqual(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + TESTUSERS(1).username.split("/")(1) + " -- as user -- 403 access denied") {
Expand Down Expand Up @@ -294,26 +285,26 @@ class TestGetUserRoute extends AnyFunSuite with BeforeAndAfterAll {
assert(response.code === HttpCode.ACCESS_DENIED.intValue)
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + "iamapikey -- as user -- 200 success, retuns self w/o password") {
test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + "iamapikey -- as user -- 200 success, retuns self") {
val response: HttpResponse[String] = Http(URL + TESTORGS(0).orgId + ROUTE + "iamapikey").headers(ACCEPT).headers(ORG1USERAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
assert(response.code === HttpCode.OK.intValue)
val responseBody: GetUsersResponse = JsonMethods.parse(response.body).extract[GetUsersResponse]
assert(responseBody.users.size === 1)
assert(responseBody.users.contains(TESTUSERS(2).username))
assertUsersEqualNoPass(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
assertUsersEqual(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + "iamtoken -- as user -- 200 success, retuns self w/o password") {
test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + "iamtoken -- as user -- 200 success, retuns self") {
val response: HttpResponse[String] = Http(URL + TESTORGS(0).orgId + ROUTE + "iamtoken").headers(ACCEPT).headers(ORG1USERAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
assert(response.code === HttpCode.OK.intValue)
val responseBody: GetUsersResponse = JsonMethods.parse(response.body).extract[GetUsersResponse]
assert(responseBody.users.size === 1)
assert(responseBody.users.contains(TESTUSERS(2).username))
assertUsersEqualNoPass(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
assertUsersEqual(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
}

}
Expand Down
23 changes: 7 additions & 16 deletions src/test/scala/com/horizon/exchangeapi/route/user/TestGetUsersRoute.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,15 +170,6 @@ class TestGetUsersRoute extends AnyFunSuite with BeforeAndAfterAll {
}

def assertUsersEqual(user1: User, user2: UserRow): Unit = {
assert(user1.password === user2.hashedPw)
assert(user1.admin === user2.admin)
assert(user1.hubAdmin === user2.hubAdmin)
assert(user1.email === user2.email)
assert(user1.lastUpdated === user2.lastUpdated)
assert(user1.updatedBy === user2.updatedBy)
}

def assertUsersEqualNoPass(user1: User, user2: UserRow): Unit = {
assert(user1.password === StrConstants.hiddenPw)
assert(user1.admin === user2.admin)
assert(user1.hubAdmin === user2.hubAdmin)
Expand All @@ -205,7 +196,7 @@ class TestGetUsersRoute extends AnyFunSuite with BeforeAndAfterAll {
assert(responseBody.users.isEmpty)
}

test("GET /orgs/root" + ROUTE + " -- as root user -- 200 success, all users in root org returned w/ hashed passwords") {
test("GET /orgs/root" + ROUTE + " -- as root user -- 200 success, all users in root org returned") {
val response: HttpResponse[String] = Http(URL + "root" + ROUTE).headers(ACCEPT).headers(ROOTAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
Expand All @@ -217,7 +208,7 @@ class TestGetUsersRoute extends AnyFunSuite with BeforeAndAfterAll {
assertUsersEqual(responseBody.users(TESTUSERS(0).username), TESTUSERS(0))
}

test("GET /orgs/root" + ROUTE + " -- as hub admin -- 200 success, all admins in root org returned w/ hashed passwords") {
test("GET /orgs/root" + ROUTE + " -- as hub admin -- 200 success, all admins in root org returned") {
val response: HttpResponse[String] = Http(URL + "root" + ROUTE).headers(ACCEPT).headers(HUBADMINAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
Expand All @@ -236,7 +227,7 @@ class TestGetUsersRoute extends AnyFunSuite with BeforeAndAfterAll {
assert(response.code === HttpCode.ACCESS_DENIED.intValue)
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + " -- as root -- 200 success, all users in org returned w/ hashed passwords") {
test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + " -- as root -- 200 success, all users in org returned") {
val response: HttpResponse[String] = Http(URL + TESTORGS(0).orgId + ROUTE).headers(ACCEPT).headers(ROOTAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
Expand All @@ -249,7 +240,7 @@ class TestGetUsersRoute extends AnyFunSuite with BeforeAndAfterAll {
assertUsersEqual(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + " -- as hub admin -- 200 success, only admins in org returned w/ hashed passwords") {
test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + " -- as hub admin -- 200 success, only admins in org returned") {
val response: HttpResponse[String] = Http(URL + TESTORGS(0).orgId + ROUTE).headers(ACCEPT).headers(HUBADMINAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
Expand All @@ -260,7 +251,7 @@ class TestGetUsersRoute extends AnyFunSuite with BeforeAndAfterAll {
assertUsersEqual(responseBody.users(TESTUSERS(1).username), TESTUSERS(1))
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + " -- as org admin -- 200 success, all users in org returned w/o passwords") {
test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + " -- as org admin -- 200 success, all users in org returned") {
val response: HttpResponse[String] = Http(URL + TESTORGS(0).orgId + ROUTE).headers(ACCEPT).headers(ORG1ADMINAUTH).asString
info("Code: " + response.code)
info("Body: " + response.body)
Expand All @@ -269,8 +260,8 @@ class TestGetUsersRoute extends AnyFunSuite with BeforeAndAfterAll {
assert(responseBody.users.size === 2)
assert(responseBody.users.contains(TESTUSERS(1).username))
assert(responseBody.users.contains(TESTUSERS(2).username))
assertUsersEqualNoPass(responseBody.users(TESTUSERS(1).username), TESTUSERS(1))
assertUsersEqualNoPass(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
assertUsersEqual(responseBody.users(TESTUSERS(1).username), TESTUSERS(1))
assertUsersEqual(responseBody.users(TESTUSERS(2).username), TESTUSERS(2))
}

test("GET /orgs/" + TESTORGS(0).orgId + ROUTE + " -- as user -- 403 access denied") {
Expand Down

0 comments on commit 60c84b1

Please sign in to comment.