GNOME Bugzilla – Bug 683437
smartcard support
Last modified: 2013-08-19 01:41:46 UTC
The fallback greeter has a plugin system and a plugin to support smartcards. We lost the plugin system, but we need the smartcard support back.
Created attachment 250400 [details] [review] loginDialog: fix up cancel button visibility This commit makes sure we hide when there's nothing to cancel to and show it when there's something to cancel to.
Created attachment 250401 [details] [review] authPrompt: disassociate from userVerifier when destroyed Otherwise, it won't get GC'd and we'll end up potentially calling its signal handlers after destruction.
Created attachment 250402 [details] [review] authPrompt: don't muck with cancelButton in onAskQuestion onAskQuestion has this code: if (this.verifyingUser) this.cancelButton.show(); else this.cancelButton.hide(); but onAskQuestion can only be called when this.verifyingUser is true. Also, cancelButton is public, and it only ever otherwise gets hidden from callers. This commit drops mucking with cancelButton visibility, leaving it entirely up to the callers to deal with.
Created attachment 250403 [details] [review] loginDialog: don't ever call _reset directly The only time we ever call _reset directly is when detecting changes to disable-user-list. We can implicitly trigger a reset for this case, just as easily by calling this._authPrompt.reset() This commit makes that change for consistency and to make it easier to adjust the authprompt workflow later.
Created attachment 250404 [details] [review] authPrompt: consolidate verifyingUser/userVerified Right now we have two booleans that specify when user verification is happening and when it succeeded, respectively. This commit consolidates them into one AuthPromptStatus enumeration. This clean up will allow us to check for verification failure more easily.
Created attachment 250405 [details] [review] util: clear user verifier after cancelling it If we don't clear it, then the connection to gdm will remain open.
Created attachment 250406 [details] [review] authPrompt: cancel user verification if verifying when reset authPrompt.reset() currently only leaves the authPrompt in a sane state if the user isn't verifying. This commit makes sure to cancel verification if a reset happens while verification is in process.
Created attachment 250407 [details] [review] unlockDialog: only emit 'failed' on reset after failure/cancel We currently emit "failed" any time the UserVerifier is reset, and user verification didn't succeed prior. A more conceptually clear time to emit "failed" would be if the UserVerifier is reset and user verification failed prior, and to emit "failed" if the user cancels unlock. This commit restructures things to do that. Aside from being more conceptually clear, it also lays the groundwork for us to be able to reset the unlock screen without failing.
Created attachment 250408 [details] [review] authPrompt: add support for auth without username This commit introduces a new BeginRequestType enum which gets passed to the 'call-begin-when-ready' signal to specify whether a username should be provided to the begin() method and changes the loginDialog to comply. Currently, the signal only ever gets emitted with AuthPrompt.BeginRequestType.PROVIDE_USERNAME but that will change in the future when providing smartcard support.
Created attachment 250409 [details] [review] util: abstract out main auth service in code Right now, the primary way a user logs in is with a password. They can also swipe their finger, if their fingerprint is enrolled, but it's expected the fingerprint auth service won't ask questions the user has to respond to by typing. As such, we ignore questions that comes from anything but the main auth service: gdm-password. In the future, if a user inserts a smartcard, we'll want to treat the gdm-smartcard service as the main auth service, and let any questions from it get to the user. This commit tries to prepare for that eventuality by storing the name of the main auth service away in a _mainService variable when verification is first begun, and then later checking incoming queries against that service instead of checking against gdm-password directly. Of course, right now, _mainService is always gdm-password.
Created attachment 250410 [details] [review] gdmUtil: pave way for fingeprint to optionally be default auth service Currently, fingerprint authentication is always a secondary thing. If a user wants to swipe their finger when the computer is asking for a password, so be it. This commit paves the way for making fingerprint auth optionally be the main way to authenticate. Currently there's no way to enable this, but in a future commit will honor enable-password-authentication=false in gsettings.
Created attachment 250411 [details] [review] authPrompt: emit prompted when given a message Some pam modules prompt without expecting the user to type an answer back (e.g. "Please swipe finger"). We need to emit prompted in this case too, so the the dialog will get shown.
Created attachment 250412 [details] [review] gdmUtil: support disabling password authentication This commit skips trying password authentication if it's disallowed, favoring fingerprint login instead.
Created attachment 250413 [details] [review] misc: add objectManager class The D-Bus ObjectManager interface is fairly recent addition to the D-Bus specification. Its purpose is to provide a standardized way to track objects dynamically coming and going for a service, and to track capabilities dynamically coming and going for those objects (by means of interfaces). This commit adds the requisite code needed to make use of the ObjectManager interface. It will ultimately be needed to implement smartcard support in the login screen.
Created attachment 250414 [details] [review] misc: add code to use settings-daemon smartcard service gnome-settings-daemon monitors smartcard insertion and removal events on the system and then exports a model of the current smartcard topology over the bus using the D-Bus ObjectManager interface. This commit adds the support code needed in gnome-shell to talk to the gnome-settings-daemon service. A future commit will use this code to inform the login screen when a user inserts a smartcard (so it can react appropriately)
Created attachment 250415 [details] [review] authPrompt: support smartcard authentication This commit detects when a user inserts a smartcard, and then initiates user verification using the gdm-smartcard PAM service. Likewise, if a user removes their smartcard, password verification (or the user list depending on auth mode and configuration) are initiated
Review of attachment 250410 [details] [review]: ::: js/gdm/util.js @@ +385,3 @@ } else if (this.serviceIsForeground(serviceName)) { this._queueMessage(info, 'login-dialog-message-info'); } This commit was supposed to flip the if and else in this conditional, but it got dropped when i was rebasing the set on top of some changes.
Created attachment 250424 [details] [review] gdmUtil: pave way for fingeprint to optionally be default auth service Currently, fingerprint authentication is always a secondary thing. If a user wants to swipe their finger when the computer is asking for a password, so be it. This commit paves the way for making fingerprint auth optionally be the main way to authenticate. Currently there's no way to enable this, but in a future commit will honor enable-password-authentication=false in gsettings.
Review of attachment 250400 [details] [review]: The style I've been using now is something more sync-like, which prevents the state from "floating in the air", so to speak. I'd prefer something like that, where we set this._authPrompt.cancelButton.visible based on explicit state in one method, and then call that everywhere we do a state transition. This means that we don't get messed up with a missing show/hide or something like that.
Review of attachment 250401 [details] [review]: I had no idea about disconnectAll. Huh.
Review of attachment 250402 [details] [review]: OK.
Review of attachment 250402 [details] [review]: (I'm not sure about "onAskQuestion can only be called when this.verifyingUser is true"; seems like that depends on the PAM stack, but OK)
Review of attachment 250403 [details] [review]: OK.
Review of attachment 250404 [details] [review]: OK.
Review of attachment 250405 [details] [review]: OK.
Review of attachment 250406 [details] [review]: ::: js/gdm/authPrompt.js @@ +462,1 @@ this.reset(); Can you remove this function now that it's synonymous with reset?
Review of attachment 250407 [details] [review]: ::: js/gdm/authPrompt.js @@ +426,1 @@ this.emit('reset'); Hm, can we remove the 'reset' signal now? I wonder if VERIFICATION_CANCELLED should be a state, too.
Attachment 250401 [details] pushed as 69957da - authPrompt: disassociate from userVerifier when destroyed Attachment 250403 [details] pushed as 925eaa1 - loginDialog: don't ever call _reset directly Attachment 250404 [details] pushed as 4d72bfd - authPrompt: consolidate verifyingUser/userVerified Attachment 250405 [details] pushed as 45ba07c - util: clear user verifier after cancelling it
(In reply to comment #22) > (I'm not sure about "onAskQuestion can only be called when this.verifyingUser > is true"; seems like that depends on the PAM stack, but OK) Nah, we set verifyingUser to true in begin when starting PAM conversation and set it to false on reset (and of course onAskQuestion only gets called when the PAM conversation is going)
(In reply to comment #30) > (In reply to comment #22) > > (I'm not sure about "onAskQuestion can only be called when this.verifyingUser > > is true"; seems like that depends on the PAM stack, but OK) > Nah, we set verifyingUser to true in begin when starting PAM conversation and > set it to false on reset (and of course onAskQuestion only gets called when the > PAM conversation is going) Ah, I thought "verifyingUser" meant "are we asking for the username?"
Created attachment 250735 [details] [review] loginDialog: fix up cancel button visibility This commit makes sure we hide when there's nothing to cancel to and show it when there's something to cancel to.
(In reply to comment #27) > Review of attachment 250406 [details] [review]: > > ::: js/gdm/authPrompt.js > @@ +462,1 @@ > this.reset(); > > Can you remove this function now that it's synonymous with reset? It's only temporarily synonymous with reset. in the commit after it becomes "reset + cancelled signal".
(In reply to comment #28) > Review of attachment 250407 [details] [review]: > > ::: js/gdm/authPrompt.js > @@ +426,1 @@ > this.emit('reset'); > > Hm, can we remove the 'reset' signal now? I wonder if VERIFICATION_CANCELLED > should be a state, too. We end up using 'reset' in a later patch. The point of these buildup patches are to separate reset from failure, so we can reset the unlock screen without forcing the curtain to come down. A VERIFICATION_CANCELLED state might make sense, though right now cancel and failure both mean the same thing "put the curtain down"
Review of attachment 250735 [details] [review]: i'll cut the gvc stuff out.
(In reply to comment #31) > Ah, I thought "verifyingUser" meant "are we asking for the username?" It (well it's inverse) kind of means that, because when we aren't running the pam conversation we are asking for a username (either via the user list or the auth prompt entry)
Review of attachment 250735 [details] [review]: ::: js/gdm/loginDialog.js @@ +540,3 @@ + // Hide the cancel button if the user list is disabled and we're asking for + // a username + if (this._authPrompt.verificationStatus == AuthPrompt.AuthPromptStatus.NOT_VERIFYING && this._disableUserList) let cancelVisible = (this._authPrompt.verificationStatus == AuthPrompt.AuthPromptStatus.NOT_VERIFYING && this._disableUserList);
Attachment 250402 [details] pushed as 58ca6ec - authPrompt: don't muck with cancelButton in onAskQuestion Attachment 250735 [details] pushed as bb2599e - loginDialog: fix up cancel button visibility
Review of attachment 250411 [details] [review]: OK.
Review of attachment 250412 [details] [review]: ::: js/gdm/util.js @@ +119,2 @@ this._settings = new Gio.Settings({ schema: LOGIN_SCREEN_SCHEMA }); + this._settings.connect('changed', Lang.bind(this, this._updateDefaultService) @@ +321,3 @@ + this._defaultService = PASSWORD_SERVICE_NAME; + else if (this._settings.get_boolean(FINGERPRINT_AUTHENTICATION_KEY)) + this._defaultService = FINGERPRINT_SERVICE_NAME; Shouldn't we check _haveFingerprintReader? @@ +344,3 @@ })); + if (this._haveFingerprintReader && !this.serviceIsForeground(FINGERPRINT_SERVICE_NAME)) { I can't help but wonder if this should be a for loop. let services = []; if (this._havePassword) services.push(PASSWORD); if (this._haveFingerprintReader) services.push(FINGERPRINT); if (this._haveSmartcardReader) services.push(SMARTCARD); services.forEach(Lang.bind(this, function(serviceName) { this._userVerification.call_begin_verification_for_user(...); })); I can't see why we care about the foreground service, other than a way of not beginning verification for the password service if it's not enabled.
Review of attachment 250409 [details] [review]: Commit message mentions _mainService, but code mentions _defaultService. ::: js/gdm/util.js @@ +305,3 @@ + _getForegroundService: function() { + return this._defaultService; Seems a bit excessive to have a member variable, a function getter (with a different naming scheme!), and then a function checker. @@ +308,3 @@ + }, + + serviceIsForeground: function(serviceName) { Does this have to be public?
Review of attachment 250408 [details] [review]: Sounds good.
Review of attachment 250408 [details] [review]: OK.
Review of attachment 250424 [details] [review]: Well, regardless of whether we make it the foreground auth service, we probably shouldn't show the user strings with "UPEK" in them. I'd rather keep that special case around.
Review of attachment 250414 [details] [review]: ::: js/misc/smartcardManager.js @@ +48,3 @@ + knownInterfaces: [ SmartcardManagerIface, + SmartcardTokenIface, + SmartcardDriverIface ], You never seem to care about Manager or Driver. @@ +51,3 @@ + onLoaded: Lang.bind(this, this._onLoaded) }); + this._insertedTokens = {}; + this._removedTokens = {}; You don't seem to use this in any meaningful way. @@ +84,3 @@ + + if (token.UsedToLogin) + this._loginToken = token; Hm, could UsedToLogin transition from true to false at runtime, and how should we handle that case? @@ +111,3 @@ + let objectPath = token.get_object_path(); + + if (objectPath) { Could this be the case, ever? @@ +122,3 @@ + } + + token.disconnectAll(); Don't you need to check if it's the _loginToken and reset it in that case?
Review of attachment 250413 [details] [review]: ::: js/misc/objectManager.js @@ +36,3 @@ + + this._connection = params.connection; + this._serviceName = params.name; this._busName = params.name; @@ +50,3 @@ + this._objects = {}; + this._interfaces = {}; + this._pendingProxies = []; What's the point of pendingProxies? You never seem to use it. @@ +70,3 @@ + g_name: this._serviceName, + g_object_path: objectPath, + g_interface_name: interfaceName, Do you need to pass both g-interface-name and g-interface-info? @@ +185,3 @@ + let [objects] = result; + + if (Object.keys(this._interfaceInfos).length == 0) { Can't this be done before we call GetManagedObjectsRemote? @@ +194,3 @@ + + // First inhibitor is to prevent onLoaded from getting + // called until all interfaces have started being added. Aren't we guaranteed that the callback of an init_async is *never* executed synchronously? @@ +203,3 @@ + let object = objects[objectPath]; + + let interfaceNames = Object.keys(object); This needs to be getOwnPropertyNames. keys() walks the prototype chain IIRC. @@ +216,3 @@ + if (this._onLoaded) + this._onLoaded(); + } Move this code to a checkCompleted(); which is called here, above, and below. @@ +232,3 @@ + for (let i = 0; i < interfaces.length; i++) { + let info = Gio.DBusInterfaceInfo.new_for_xml(interfaces[i]); + Remove new line.
Review of attachment 250415 [details] [review]: ::: js/gdm/authPrompt.js @@ +227,3 @@ + + // Don't reset on smartcard insertion if we're already verifying + // and the smartcard is the main service Could you explain this to me in reverse? When *should* we reset on smartcard insertion / removal? @@ +450,3 @@ + if (this._mode == AuthPromptMode.UNLOCK_ONLY) { + // The user is constant at the unlock screen + beginRequestType = BeginRequestType.PROVIDE_USERNAME; This is a bit strange, but I guess it works. It should have a comment saying "the unlock screen will immediately reply with the username" or similar. ::: js/gdm/util.js @@ +136,3 @@ + this._checkForSmartcard(); + + this._smartcardManager.connect('smartcard-inserted', Lang.bind(this, this._checkForSmartcard) ::: js/ui/screenShield.js @@ +522,3 @@ + Lang.bind(this, function() { + if (this._isLocked) + this._liftShield(true, 0); Uh, shouldn't we check that it's the correct smartcard, or will we want to show the user an error on the dialog?
(In reply to comment #41) > Review of attachment 250412 [details] [review]: > @@ +321,3 @@ > + this._defaultService = PASSWORD_SERVICE_NAME; > + else if (this._settings.get_boolean(FINGERPRINT_AUTHENTICATION_KEY)) > + this._defaultService = FINGERPRINT_SERVICE_NAME; > > Shouldn't we check _haveFingerprintReader? Indeed. > @@ +344,3 @@ > I can't help but wonder if this should be a for loop. Not really. We don't want to start the password service if the smartcard service is going, and we always want to start the fingerprint service in the background unless it's the only service, then you want to run it in the foreground. > I can't see why we care about the foreground service, other than a way of not > beginning verification for the password service if it's not enabled. The foreground service is the one we show messages for and the one we receive answers for. The other two options are 1) the background service (at the moment fingerprint reader) 2) a service not running at all (e.g. password service if a smartcard is plugged in)
(In reply to comment #50) > Not really. We don't want to start the password service if the smartcard > service is going, and we always want to start the fingerprint service in the > background unless it's the only service, then you want to run it in the > foreground. Then can you at least extract the common code into a this._startService method?
(In reply to comment #42) > Review of attachment 250409 [details] [review]: > > Commit message mentions _mainService, but code mentions _defaultService. I fixed that on the branch but I didn't bother reupdating the bug since it was only a commit message change. > ::: js/gdm/util.js > @@ +305,3 @@ > > + _getForegroundService: function() { > + return this._defaultService; > > Seems a bit excessive to have a member variable, a function getter (with a > different naming scheme!), and then a function checker. It's just laying the stage to be expanded in a future commit. The commit message talks about that a bit. > @@ +308,3 @@ > + }, > + > + serviceIsForeground: function(serviceName) { > > Does this have to be public? Yea, patches later the stack end up using it. Remember this is just an intermediate patch toward the end goal. It's point is to make it easy to see the succession of changes and to help lay the stage for future changes.
(In reply to comment #51) > Then can you at least extract the common code into a this._startService method? sure.
(In reply to comment #45) > Review of attachment 250424 [details] [review]: > > Well, regardless of whether we make it the foreground auth service, we probably > shouldn't show the user strings with "UPEK" in them. I'd rather keep that > special case around. Well, three points 1) We have to show some message. We can't just put (or swipe finger) when there's nothing happening above it. 2) The situation these days isn't as dire with pam_fprintd. It doesn't say UPEK anymore anyway. 3) I don't expect "fingerprint only" is going to be a common configuration. This commit is really more about setting the stage so "smartcard only" (which is pretty common) falls out for free.
(In reply to comment #46) > Review of attachment 250414 [details] [review]: > > ::: js/misc/smartcardManager.js > @@ +48,3 @@ > + > knownInterfaces: [ SmartcardManagerIface, > + > SmartcardTokenIface, > + > SmartcardDriverIface ], > > You never seem to care about Manager or Driver. Will fix. > @@ +51,3 @@ > + onLoaded: > Lang.bind(this, this._onLoaded) }); > + this._insertedTokens = {}; > + this._removedTokens = {}; > > You don't seem to use this in any meaningful way. I used insertedTokens, but not removedTokens. Will fix. > > @@ +84,3 @@ > + > + if (token.UsedToLogin) > + this._loginToken = token; > > Hm, could UsedToLogin transition from true to false at runtime, and how should > we handle that case? Nope. It's a fact-encoded-as-boolean about how the user logged into the machine. That can't change after the fact. > @@ +111,3 @@ > + let objectPath = token.get_object_path(); > + > + if (objectPath) { > > Could this be the case, ever? objectPath will never be null. will fix. > > @@ +122,3 @@ > + } > + > + token.disconnectAll(); > > Don't you need to check if it's the _loginToken and reset it in that case? yea, probably.
(In reply to comment #48) > What's the point of pendingProxies? You never seem to use it. To root proxies that are still being initialized. It's not really necessary since the GAsyncInitable will hold a reference itself while initializing. I'll drop it. > > @@ +70,3 @@ > + g_name: this._serviceName, > + g_object_path: objectPath, > + g_interface_name: interfaceName, > > Do you need to pass both g-interface-name and g-interface-info? Not sure. At least makeProxyWrapper does under the hood for it's similar GDBusProxy usage, and the C api takes both parameters, so i think so, but haven't tried skipping g_interface_name. > > @@ +185,3 @@ > + let [objects] = result; > + > + if (Object.keys(this._interfaceInfos).length == 0) { > > Can't this be done before we call GetManagedObjectsRemote? Yea, it could be and would mean skipping the RPC call in that case. i'll change it. > @@ +194,3 @@ > + > + // First inhibitor is to prevent onLoaded from getting > + // called until all interfaces have started being added. > > Aren't we guaranteed that the callback of an init_async is *never* executed > synchronously? We ourselves, abort mission early in some cases before calling the init_async, so it doesn't matter. Even so, I'd rather not rely on those init_async semantics anyway when this removes any question and seems clearer to me. > @@ +203,3 @@ > + let object = objects[objectPath]; > + > + let interfaceNames = Object.keys(object); > > This needs to be getOwnPropertyNames. keys() walks the prototype chain IIRC. sure, will change. > @@ +216,3 @@ > + if (this._onLoaded) > + this._onLoaded(); > + } > > Move this code to a checkCompleted(); which is called here, above, and below. okay. > @@ +232,3 @@ > + for (let i = 0; i < interfaces.length; i++) { > + let info = Gio.DBusInterfaceInfo.new_for_xml(interfaces[i]); > + > > Remove new line. okay
> Review of attachment 250415 [details] [review]: > > ::: js/gdm/authPrompt.js > @@ +227,3 @@ > + > + // Don't reset on smartcard insertion if we're already verifying > + // and the smartcard is the main service > > Could you explain this to me in reverse? When *should* we reset on smartcard > insertion / removal? If we're sitting at the user list and a user inserts their smartcard we should reset. If we're sitting at a password prompt and the user inserts their smartcard we should reset. If we're sitting asking for a Username and the user inserts their smartcard we should reset. If we're asking for a smartcard pin and the user removes their smartcard we should reset. > @@ +450,3 @@ > + if (this._mode == AuthPromptMode.UNLOCK_ONLY) { > + // The user is constant at the unlock screen > + beginRequestType = BeginRequestType.PROVIDE_USERNAME; > > This is a bit strange, but I guess it works. It should have a comment saying > "the unlock screen will immediately reply with the username" or similar. sure. > Uh, shouldn't we check that it's the correct smartcard, or will we want to show > the user an error on the dialog? probably better to check if it's the correct smartcard, i'll change it.
Created attachment 251856 [details] [review] authPrompt: cancel user verification if verifying when reset authPrompt.reset() currently only leaves the authPrompt in a sane state if the user isn't verifying. This commit makes sure to cancel verification if a reset happens while verification is in process.
Created attachment 251857 [details] [review] unlockDialog: only emit 'failed' on reset after failure/cancel We currently emit "failed" any time the UserVerifier is reset, and user verification didn't succeed prior. A more conceptually clear time to emit "failed" would be if the UserVerifier is reset and user verification failed prior, and to emit "failed" if the user cancels unlock. This commit restructures things to do that. Aside from being more conceptually clear, it also lays the groundwork for us to be able to reset the unlock screen without failing.
Created attachment 251858 [details] [review] authPrompt: add support for auth without username This commit introduces a new BeginRequestType enum which gets passed to the 'reset' signal to specify whether a username should be provided to the begin() method and changes the loginDialog to comply. Currently, the signal only ever gets emitted with AuthPrompt.BeginRequestType.PROVIDE_USERNAME but that will change in the future when providing smartcard support.
Created attachment 251859 [details] [review] util: abstract out default auth service in code Right now, the primary way a user logs in is with a password. They can also swipe their finger, if their fingerprint is enrolled, but it's expected the fingerprint auth service won't ask questions the user has to respond to by typing. As such, we ignore questions that comes from anything but the main auth service: gdm-password. In the future, if a user inserts a smartcard, we'll want to treat the gdm-smartcard service as the main auth service, and let any questions from it get to the user. This commit tries to prepare for that eventuality by storing the name of the default auth service away in a _defaultService variable before verification has begun, and then later checking incoming queries against that service instead of checking against string 'gdm-password' directly. Of course, right now, _defaultService is always gdm-password.
Created attachment 251860 [details] [review] gdmUtil: pave way for fingeprint to optionally be default auth service Currently, fingerprint authentication is always a secondary thing. If a user wants to swipe their finger when the computer is asking for a password, so be it. This commit paves the way for making fingerprint auth optionally be the main way to authenticate. Currently there's no way to enable this, but in a future commit will honor enable-password-authentication=false in gsettings.
Created attachment 251861 [details] [review] authPrompt: emit prompted when given a message Some pam modules prompt without expecting the user to type an answer back (e.g. "Please swipe finger"). We need to emit prompted in this case too, so the the dialog will get shown.
Created attachment 251862 [details] [review] gdmUtil: factor out some duplicated code in beginVerification The duplication makes the function look a lot more complicated than it actually is. This commit moves the common code to a new _startService function.
Created attachment 251863 [details] [review] gdmUtil: support disabling password authentication This commit skips trying password authentication if it's disallowed, favoring fingerprint login instead.
Created attachment 251864 [details] [review] misc: add objectManager class The D-Bus ObjectManager interface is fairly recent addition to the D-Bus specification. Its purpose is to provide a standardized way to track objects dynamically coming and going for a service, and to track capabilities dynamically coming and going for those objects (by means of interfaces). This commit adds the requisite code needed to make use of the ObjectManager interface. It will ultimately be needed to implement smartcard support in the login screen.
Created attachment 251865 [details] [review] misc: add code to use settings-daemon smartcard service gnome-settings-daemon monitors smartcard insertion and removal events on the system and then exports a model of the current smartcard topology over the bus using the D-Bus ObjectManager interface. This commit adds the support code needed in gnome-shell to talk to the gnome-settings-daemon service. A future commit will use this code to inform the login screen when a user inserts a smartcard (so it can react appropriately)
Created attachment 251866 [details] [review] authPrompt: support smartcard authentication This commit detects when a user inserts a smartcard, and then initiates user verification using the gdm-smartcard PAM service. Likewise, if a user removes their smartcard, password verification (or the user list depending on auth mode and configuration) are initiated
Review of attachment 251856 [details] [review]: OK.
Review of attachment 251857 [details] [review]: OK.
Review of attachment 251859 [details] [review]: OK.
Review of attachment 251860 [details] [review]: OK. ::: js/gdm/util.js @@ +381,2 @@ this._haveFingerprintReader) { + // We don't show fingeprint messages directly since it's typo: "fingeprint"
Review of attachment 251862 [details] [review]: > 1 file changed, 21 insertions(+), 52 deletions(-) Nice.
Review of attachment 251863 [details] [review]: OK.
Review of attachment 251864 [details] [review]: Much better. ::: js/misc/objectManager.js @@ +86,3 @@ + proxy.init_async(GLib.PRIORITY_DEFAULT, + this._cancellable, + Lang.bind(this, function(initable, result) { Would be nice if this function was indented like the one below -- four spaces from the previous indentation, not the paren.
Review of attachment 251865 [details] [review]: ::: js/misc/smartcardManager.js @@ +30,3 @@ + _init: function() { + this._objectManager = new ObjectManager.ObjectManager({ connection: Gio.DBus.session, + name: _SMARTCARD_SERVICE_DBUS_NAME, There's no need for this to be a constant; just embed it inline. @@ +44,3 @@ + this._addToken(tokens[i]); + + this._objectManager.connect('interface-added', Lang.bind(this, function(objectManager, interfaceName, proxy) { Ditto with all these functions as well.
Review of attachment 251866 [details] [review]: OK.
Attachment 251856 [details] pushed as f5b2feb - authPrompt: cancel user verification if verifying when reset Attachment 251857 [details] pushed as 1104a38 - unlockDialog: only emit 'failed' on reset after failure/cancel Attachment 251858 [details] pushed as 93f072d - authPrompt: add support for auth without username Attachment 251859 [details] pushed as 148f221 - util: abstract out default auth service in code Attachment 251860 [details] pushed as a2a5f5d - gdmUtil: pave way for fingeprint to optionally be default auth service Attachment 251861 [details] pushed as 07b57de - authPrompt: emit prompted when given a message Attachment 251862 [details] pushed as fd11ad9 - gdmUtil: factor out some duplicated code in beginVerification Attachment 251863 [details] pushed as 4394a05 - gdmUtil: support disabling password authentication Attachment 251864 [details] pushed as 4b450ba - misc: add objectManager class Attachment 251865 [details] pushed as 35fa42c - misc: add code to use settings-daemon smartcard service Attachment 251866 [details] pushed as 059b75c - authPrompt: support smartcard authentication