Merge pull request #1324 from wireapp/release_2021_01_12

Release 2020-01-12

CHANGELOG.md

@@ -14,6 +14,24 @@
 
 -->
 
+# [2021-01-12]
+
+## Release Notes
+
+This release contains bugfixes and internal changes.
+
+## Bug fixes and other updates
+
+* [SCIM] Fix bug: Deleting a user retains their externalId (#1323)
+* [SCIM] Fix bug: Provisioned users can update update to email, handle, name (#1320)
+
+## Internal changes
+
+* [SCIM] Add logging to SCIM ops, invitation ops, createUser (#1322) (#1318)
+* Upgrade nixpkgs and add HLS to shell.nix (#1314)
+* create_test_team_scim.sh script: fix arg parsing and invite (#1321)
+
+
 # [2021-01-06]
 
 ## Release Notes

deploy/services-demo/create_test_team_scim.sh

@@ -23,7 +23,7 @@ USAGE: $0
 
 # Option parsing:
 # https://sookocheff.com/post/bash/parsing-bash-script-arguments-with-shopts/
-while getopts ":n:h:c" opt; do
+while getopts ":h:s:" opt; do
   case ${opt} in
     h ) BRIG_HOST="$OPTARG"
       ;;
@@ -182,9 +182,13 @@ REGISTER_ACCEPT=$(cat <<EOF
 EOF
 )
 
-SCIM_USER_REGISTER_TEAM=$(curl -XPOST "$BRIG_HOST/register" \
-     --header 'Content-Type: application/json' \
-     -d "$REGISTER_ACCEPT" | jq -r .team)
+# Create the user using that code
+CURL_OUT=$(curl \
+            -XPOST "$BRIG_HOST/i/users" \
+            -H'Content-type: application/json' \
+            -d'{"email":"'"$scimUserEmail"'","password":"'"$scimUserPassword"'","name":"'"$scimUserDisplayName"'","team_code":"'"$SCIM_USER_INVITATION_CODE"'"}')
+
+SCIM_USER_REGISTER_TEAM=$(echo "$CURL_OUT" | jq -r .team)
 
 if [ "$SCIM_USER_REGISTER_TEAM" != "$TEAM_UUID" ]; then
     echo "unexpected error: user got assigned to no / the wrong team?!"

libs/types-common/package.yaml

@@ -26,6 +26,7 @@ library:
   - containers >=0.5
   - cryptohash-md5 >=0.11.7.2
   - cryptohash-sha1 >=0.11.7.2
+  - cryptonite >=0.26
   - data-default >=0.5
   - deepseq >=1.4
   - directory >=1.2
@@ -54,6 +55,7 @@ library:
   - text >=0.11
   - time >=1.6
   - time-locale-compat >=0.1
+  - tinylog >=0.14
   - transformers >=0.3
   - unix
   - unordered-containers >=0.2

libs/types-common/src/Util/Logging.hs

@@ -0,0 +1,51 @@
+-- This file is part of the Wire Server implementation.
+--
+-- Copyright (C) 2020 Wire Swiss GmbH <[email protected]>
+--
+-- This program is free software: you can redistribute it and/or modify it under
+-- the terms of the GNU Affero General Public License as published by the Free
+-- Software Foundation, either version 3 of the License, or (at your option) any
+-- later version.
+--
+-- This program is distributed in the hope that it will be useful, but WITHOUT
+-- ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+-- FOR A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
+-- details.
+--
+-- You should have received a copy of the GNU Affero General Public License along
+-- with this program. If not, see <https://www.gnu.org/licenses/>.
+
+module Util.Logging where
+
+import Crypto.Hash (SHA256, hash)
+import Data.Handle (Handle (fromHandle))
+import Data.Id (TeamId, UserId)
+import Data.String.Conversions (cs)
+import qualified Data.Text as T
+import Data.Text.Encoding (encodeUtf8)
+import Imports
+import qualified System.Logger.Class as Log
+import System.Logger.Message (Msg)
+
+sha256String :: Text -> Text
+sha256String t =
+  let digest = hash @ByteString @SHA256 (encodeUtf8 t)
+   in cs . show $ digest
+
+logHandle :: Handle -> (Msg -> Msg)
+logHandle handl =
+  Log.field "handle_sha256" (sha256String . fromHandle $ handl)
+
+logFunction :: Text -> (Msg -> Msg)
+logFunction fn = Log.field "fn" fn . Log.field "module" (getModule fn)
+  where
+    getModule :: Text -> Text
+    getModule t = case T.split (== '.') t of
+      [] -> ""
+      x -> T.intercalate "." (init x)
+
+logUser :: UserId -> (Msg -> Msg)
+logUser uid = Log.field "user" (cs @_ @Text . show $ uid)
+
+logTeam :: TeamId -> (Msg -> Msg)
+logTeam tid = Log.field "team" (cs @_ @Text . show $ tid)

libs/types-common/types-common.cabal

@@ -4,7 +4,7 @@ cabal-version: 1.12
 --
 -- see: https://github.com/sol/hpack
 --
--- hash: ee2619b6133e11f5de7ace09995fa308d2c5c6ecc46e81b8e10283594a7aef26
+-- hash: 74a96e1b02e343b15dc21c298af9a133f9e26b7e36335a91473acc75f4dff648
 
 name:           types-common
 version:        0.16.0
@@ -37,6 +37,7 @@ library
       Data.Text.Ascii
       Data.UUID.Tagged
       Util.Attoparsec
+      Util.Logging
       Util.Options
       Util.Options.Common
       Util.Test
@@ -61,6 +62,7 @@ library
     , containers >=0.5
     , cryptohash-md5 >=0.11.7.2
     , cryptohash-sha1 >=0.11.7.2
+    , cryptonite >=0.26
     , data-default >=0.5
     , deepseq >=1.4
     , directory >=1.2
@@ -89,6 +91,7 @@ library
     , text >=0.11
     , time >=1.6
     , time-locale-compat >=0.1
+    , tinylog >=0.14
     , transformers >=0.3
     , unix
     , unordered-containers >=0.2

nix/sources.json

@@ -17,10 +17,10 @@
         "homepage": "https://github.com/NixOS/nixpkgs",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "502845c3e31ef3de0e424f3fcb09217df2ce6df6",
-        "sha256": "0fcqpsy6y7dgn0y0wgpa56gsg0b0p8avlpjrd79fp4mp9bl18nda",
+        "rev": "2080afd039999a58d60596d04cefb32ef5fcc2a2",
+        "sha256": "0i677swvj8fxfwg3jibd0xl33rn0rq0adnniim8jnp384whnh8ry",
         "type": "tarball",
-        "url": "https://github.com/NixOS/nixpkgs-channels/archive/502845c3e31ef3de0e424f3fcb09217df2ce6df6.tar.gz",
+        "url": "https://github.com/NixOS/nixpkgs/archive/2080afd039999a58d60596d04cefb32ef5fcc2a2.tar.gz",
         "url_template": "https://github.com/<owner>/<repo>/archive/<rev>.tar.gz"
     }
 }

services/brig/src/Brig/API/Error.hs

@@ -116,6 +116,7 @@ changeEmailError :: ChangeEmailError -> Error
 changeEmailError (InvalidNewEmail _ _) = StdError invalidEmail
 changeEmailError (EmailExists _) = StdError userKeyExists
 changeEmailError (ChangeBlacklistedEmail _) = StdError blacklistedEmail
+changeEmailError EmailManagedByScim = StdError $ propertyManagedByScim "email"
 
 changePhoneError :: ChangePhoneError -> Error
 changePhoneError (InvalidNewPhone _) = StdError invalidPhone
@@ -130,6 +131,7 @@ changeHandleError :: ChangeHandleError -> Error
 changeHandleError ChangeHandleNoIdentity = StdError (noIdentity 2)
 changeHandleError ChangeHandleExists = StdError handleExists
 changeHandleError ChangeHandleInvalid = StdError invalidHandle
+changeHandleError ChangeHandleManagedByScim = StdError $ propertyManagedByScim "handle"
 
 legalHoldLoginError :: LegalHoldLoginError -> Error
 legalHoldLoginError LegalHoldLoginNoBindingTeam = StdError noBindingTeam
@@ -207,6 +209,10 @@ phoneError PhoneNumberUnreachable = StdError invalidPhone
 phoneError PhoneNumberBarred = StdError blacklistedPhone
 phoneError (PhoneBudgetExhausted t) = RichError phoneBudgetExhausted (PhoneBudgetTimeout t) []
 
+updateProfileError :: UpdateProfileError -> Error
+updateProfileError DisplayNameManagedByScim = StdError (propertyManagedByScim "name")
+updateProfileError (ProfileNotFound _) = StdError userNotFound
+
 -- WAI Errors -----------------------------------------------------------------
 
 tooManyProperties :: Wai.Error
@@ -427,6 +433,9 @@ insufficientTeamPermissions = Wai.Error status403 "insufficient-permissions" "In
 noBindingTeam :: Wai.Error
 noBindingTeam = Wai.Error status403 "no-binding-team" "Operation allowed only on binding teams"
 
+propertyManagedByScim :: LText -> Wai.Error
+propertyManagedByScim prop = Wai.Error status403 "managed-by-scim" $ "Updating \"" <> prop <> "\" is not allowed, because it is managed by SCIM"
+
 sameBindingTeamUsers :: Wai.Error
 sameBindingTeamUsers = Wai.Error status403 "same-binding-team-users" "Operation not allowed to binding team users."
 

services/brig/src/Brig/API/Internal.hs

@@ -322,17 +322,17 @@ deleteUserNoVerify uid = do
 changeSelfEmailMaybeSendH :: UserId ::: Bool ::: JsonRequest EmailUpdate -> Handler Response
 changeSelfEmailMaybeSendH (u ::: validate ::: req) = do
   email <- euEmail <$> parseJsonBody req
-  changeSelfEmailMaybeSend u (if validate then ActuallySendEmail else DoNotSendEmail) email >>= \case
+  changeSelfEmailMaybeSend u (if validate then ActuallySendEmail else DoNotSendEmail) email API.AllowSCIMUpdates >>= \case
     ChangeEmailResponseIdempotent -> pure (setStatus status204 empty)
     ChangeEmailResponseNeedsActivation -> pure (setStatus status202 empty)
 
 data MaybeSendEmail = ActuallySendEmail | DoNotSendEmail
 
-changeSelfEmailMaybeSend :: UserId -> MaybeSendEmail -> Email -> Handler ChangeEmailResponse
-changeSelfEmailMaybeSend u ActuallySendEmail email = do
-  API.changeSelfEmail u email
-changeSelfEmailMaybeSend u DoNotSendEmail email = do
-  API.changeEmail u email !>> changeEmailError >>= \case
+changeSelfEmailMaybeSend :: UserId -> MaybeSendEmail -> Email -> API.AllowSCIMUpdates -> Handler ChangeEmailResponse
+changeSelfEmailMaybeSend u ActuallySendEmail email allowScim = do
+  API.changeSelfEmail u email allowScim
+changeSelfEmailMaybeSend u DoNotSendEmail email allowScim = do
+  API.changeEmail u email allowScim !>> changeEmailError >>= \case
     ChangeEmailIdempotent -> pure ChangeEmailResponseIdempotent
     ChangeEmailNeedsActivation _ -> pure ChangeEmailResponseNeedsActivation
 
@@ -518,7 +518,7 @@ updateHandleH (uid ::: _ ::: body) = empty <$ (updateHandle uid =<< parseJsonBod
 updateHandle :: UserId -> HandleUpdate -> Handler ()
 updateHandle uid (HandleUpdate handleUpd) = do
   handle <- validateHandle handleUpd
-  API.changeHandle uid Nothing handle !>> changeHandleError
+  API.changeHandle uid Nothing handle API.AllowSCIMUpdates !>> changeHandleError
 
 updateUserNameH :: UserId ::: JSON ::: JsonRequest NameUpdate -> Handler Response
 updateUserNameH (uid ::: _ ::: body) = empty <$ (updateUserName uid =<< parseJsonBody body)
@@ -534,7 +534,7 @@ updateUserName uid (NameUpdate nameUpd) = do
             uupAccentId = Nothing
           }
   lift (Data.lookupUser WithPendingInvitations uid) >>= \case
-    Just _ -> lift $ API.updateUser uid Nothing uu
+    Just _ -> API.updateUser uid Nothing uu API.AllowSCIMUpdates !>> updateProfileError
     Nothing -> throwStd invalidUser
 
 checkHandleInternalH :: Text -> Handler Response
@@ -547,7 +547,7 @@ checkHandleInternalH =
 getContactListH :: JSON ::: UserId -> Handler Response
 getContactListH (_ ::: uid) = do
   contacts <- lift $ API.lookupContactList uid
-  return $ json $ (UserIds contacts)
+  return $ json $ UserIds contacts
 
 -- Deprecated
 

services/brig/src/Brig/API/Public.hs

@@ -37,6 +37,7 @@ import Brig.API.IdMapping (resolveOpaqueUserId)
 import qualified Brig.API.Properties as API
 import Brig.API.Types
 import qualified Brig.API.User as API
+import Brig.API.Util
 import qualified Brig.API.Util as API
 import Brig.App
 import qualified Brig.Calling.API as Calling
@@ -47,7 +48,7 @@ import qualified Brig.Team.API as Team
 import qualified Brig.Team.Email as Team
 import Brig.Types.Activation (ActivationPair)
 import Brig.Types.Intra (AccountStatus (Ephemeral), UserAccount (UserAccount, accountUser))
-import Brig.Types.User (HavePendingInvitations (..))
+import Brig.Types.User (HavePendingInvitations (..), User (userId))
 import qualified Brig.User.API.Auth as Auth
 import qualified Brig.User.API.Search as Search
 import qualified Brig.User.Auth.Cookie as Auth
@@ -91,6 +92,7 @@ import Servant.Swagger (HasSwagger (toSwagger))
 import Servant.Swagger.Internal.Orphans ()
 import Servant.Swagger.UI
 import qualified System.Logger.Class as Log
+import Util.Logging (logFunction, logHandle, logTeam, logUser)
 import qualified Wire.API.Connection as Public
 import qualified Wire.API.Properties as Public
 import qualified Wire.API.Swagger as Public.Swagger (models)
@@ -1142,14 +1144,28 @@ createUser (Public.NewUserPublic new) = do
   for_ (Public.newUserPhone new) $ checkWhitelist . Right
   result <- API.createUser new !>> newUserError
   let acc = createdAccount result
-  lift $ Log.debug (Log.msg $ "createUser: acc: " <> show acc)
+
   let eac = createdEmailActivation result
   let pac = createdPhoneActivation result
   let epair = (,) <$> (activationKey <$> eac) <*> (activationCode <$> eac)
   let ppair = (,) <$> (activationKey <$> pac) <*> (activationCode <$> pac)
   let newUserLabel = Public.newUserLabel new
   let newUserTeam = Public.newUserTeam new
   let usr = accountUser acc
+
+  let context =
+        let invitationCode = case Public.newUserTeam new of
+              (Just (Public.NewTeamMember code)) -> Just code
+              _ -> Nothing
+         in ( logFunction "Brig.API.Public.createUser"
+                . logUser (Public.userId usr)
+                . maybe id logHandle (Public.userHandle usr)
+                . maybe id logTeam (Public.userTeam usr)
+                . maybe id logEmail (Public.userEmail usr)
+                . maybe id logInvitationCode invitationCode
+            )
+  Log.info $ context . Log.msg @Text "Sucessfully created user"
+
   let Public.User {userLocale, userDisplayName, userId} = usr
   let userEmail = Public.userEmail usr
   let userPhone = Public.userPhone usr
@@ -1274,7 +1290,7 @@ instance ToJSON GetActivationCodeResp where
 updateUserH :: UserId ::: ConnId ::: JsonRequest Public.UserUpdate -> Handler Response
 updateUserH (uid ::: conn ::: req) = do
   uu <- parseJsonBody req
-  lift $ API.updateUser uid (Just conn) uu
+  API.updateUser uid (Just conn) uu API.ForbidSCIMUpdates !>> updateProfileError
   return empty
 
 changePhoneH :: UserId ::: ConnId ::: JsonRequest Public.PhoneUpdate -> Handler Response
@@ -1367,7 +1383,8 @@ changeHandleH (u ::: conn ::: req) = do
 changeHandle :: UserId -> ConnId -> Public.HandleUpdate -> Handler ()
 changeHandle u conn (Public.HandleUpdate h) = do
   handle <- API.validateHandle h
-  API.changeHandle u (Just conn) handle !>> changeHandleError
+  -- TODO check here
+  API.changeHandle u (Just conn) handle API.ForbidSCIMUpdates !>> changeHandleError
 
 beginPasswordResetH :: JSON ::: JsonRequest Public.NewPasswordReset -> Handler Response
 beginPasswordResetH (_ ::: req) = do
@@ -1418,7 +1435,7 @@ customerExtensionCheckBlockedDomains email = do
 changeSelfEmailH :: UserId ::: ConnId ::: JsonRequest Public.EmailUpdate -> Handler Response
 changeSelfEmailH (u ::: _ ::: req) = do
   email <- Public.euEmail <$> parseJsonBody req
-  API.changeSelfEmail u email >>= \case
+  API.changeSelfEmail u email API.ForbidSCIMUpdates >>= \case
     ChangeEmailResponseIdempotent -> pure (setStatus status204 empty)
     ChangeEmailResponseNeedsActivation -> pure (setStatus status202 empty)
 

services/brig/src/Brig/API/Types.hs

@@ -103,6 +103,10 @@ data CreateUserError
   | -- | Some precondition on another Wire service failed. We propagate this error.
     ExternalPreconditionFailed Wai.Error
 
+data UpdateProfileError
+  = DisplayNameManagedByScim
+  | ProfileNotFound UserId
+
 data InvitationError
   = InviteeEmailExists UserId
   | InviteInvalidEmail Email
@@ -163,11 +167,13 @@ data ChangeEmailError
   = InvalidNewEmail !Email !String
   | EmailExists !Email
   | ChangeBlacklistedEmail !Email
+  | EmailManagedByScim
 
 data ChangeHandleError
   = ChangeHandleNoIdentity
   | ChangeHandleExists
   | ChangeHandleInvalid
+  | ChangeHandleManagedByScim
 
 data SendActivationCodeError
   = InvalidRecipient UserKey