Closed chlowell closed 3 months ago
This is awesome!
IMHO, I would rather see the OneAuth as an external binary (instead of embedded library). Is there a reason why that approach wouldn't work?
Probably a tool on azure-sdk-tools repo (or its own repo) which publishes a 64-binary and a 64-arm binary on github. Then azd would just pull it into ~/.azd/bin the same way it does for bicep and gh-cli.
The tool would become beneficial for other cli as well, like az. And we could let azd to chose interactive-browser or OneAuth with app-configuration, instead of build-arg...
Thanks @chlowell,
This is some work that Charles did to get azd
to use a centralized auth library from an internal Microsoft team which handles some new features we might want. Part of the exercise was to figure out how we could integrate with this existing C++ library from a Go app which had challenging deployment characteristics (i.e. we can't statically link it, there's no stable C ABI).
Charles is going to take these learnings back to the OneAuth team and see if we can make some improvements to their library to make it easier for tools like us to consume.
I asked @chlowell to open this PR because the stuff is behind a build flag so there should be no impact to the standard AZD builds. I think we will want to take and merge this code (still behind a flag) and maintain it because we might need to integrate with OneAuth at one point (for one thing, it gives us an answer for WAM support, something people mention from time to time internally).
Thanks for the context @ellismg
Still curious about the "why" around making azd to consume a C++ library
instead of creating a C++ binary
which azd can consume as external dependency.
Thanks for adding context @ellismg. To add some more, this is part of an effort to unify DevDiv tools on a single authentication stack to reduce duplicate effort, unify UX, and make single sign-on possible.
Still curious about the "why" around making azd to consume a
C++ library
instead of creating aC++ binary
which azd can consume as external dependency.
The OneAuth team doesn't ship binaries because the project is for first-party use only.
The OneAuth team doesn't ship binaries because the project is for first-party use only.
@chlowell , if I understand correctly, we are introducing a go-layer to azd to call the C++ library (think we call it the bridge
). So, instead of maintaining such bridge, can we maintain a C++ binary that calls the C++ library based on input args?
Either way, looks like we would be signing to maintain the bridge
, either with the go-to-dll strategy (in-process) or by calling an external app (external-process). I would vote for an external app and stay away from the insecure
packages and manual memory management with go
.
From a C++ binary, we can use smart-pointers and keep it idiomatic.
Let me pull @JeffreyRichter and ask for advising here.
if I understand correctly, we are introducing a go-layer to azd to call the C++ library (think we call it
the bridge
). So, instead of maintaining such bridge, can we maintain a C++ binary that calls the C++ library based on input args?
Sorry, I don't understand. oneauth/bridge
is a C++ binary that calls OneAuth based on input args. What other binary should we maintain?
if I understand correctly, we are introducing a go-layer to azd to call the C++ library (think we call it
the bridge
). So, instead of maintaining such bridge, can we maintain a C++ binary that calls the C++ library based on input args?Sorry, I don't understand.
oneauth/bridge
is a C++ binary that calls OneAuth based on input args. What other binary should we maintain?
For what I'm reading in the code, we are not creating an exe
to be executed on Windows. We are creating a library (dll) and using go-code to run the code from that dll (https://github.com/Azure/azure-dev/pull/3216/files#diff-5ed96b12f86a7dea76bf70a95444375063fbcd2733842f070beccf892a9ac1caR156). That is what I am trying to understand the why
.
I am wondering if we can move the c++ code to its own repo and update the code to create an exe file (having a main() entry point for the OS to init :).
From that repo, we can produce static build (exe alone) and dynamic build (exe + dll). Or any other configuration.
Then, from azd, we would use the commandRunner
( the one we use to run external tools like bicep, dotnet, npm, etc) to invoke the c++ binary (the exe).
I will admit I encouraged @chlowell a bit down the path he went down (embedding the DLL) because I think in both of our minds where were thinking: "In an ideal world we'd have some OneAuth library we could statically link against with Cgo but the library today does not support static linking so we'll do this other thing that looks sort of like static linking if you squint and then try to push the team that owns OneAuth into building a stable C ABI and providing a library we can statically link against".
I also like the idea of azd
carrying with it everything it needs to have for auth instead of requiring it to download other binaries to do it.
That said, I could imagine that we embed a exe
instead of a dll
and then use some mechanism to invoke it (either via our command runner and a set of flags) or some other strategy like JSON-RPC but I'd want to know what we really gain by doing that. I guess azd
itself can remain pure go on Windows, which is quite nice, but I'm not actually sure it's worth it. We'd get to remove some of the interop code, but it feels like the overall architecture ends up being more complex an harder to evovle.
I also don't know if having it be a sperate process is going to be a bad idea long term, for example in cases where the component has to pop UI to log in. That UI not being associated with the azd
process (and instead some azd-auth-helper-one-auth
process) may be probmatic?
Thanks for all the feedback. I'll mark this ready to review when I think it's ready to merge. I'm still debugging MSA authentication, which looks like it should work but doesn't.
I'm still not sure I want to enable this by default in
azd
but having the code here for when we are ready to turn it on makes sense.
👍 I agree, today this is a lot of complexity for not much benefit. How would you feel about shipping it in a beta?
It would be nice to get at least some coverage of the build (I don't think we need to run tests - and also, I think running these tests in CI would be "hard"). Maybe @danieljurek can help get another leg of our pipeline that passes a path to the msys2 shell and skips the tests.
Once you've got build automation the tests should just work in CI--they don't require user interaction--provided the agent can write %LocalAppData%. Setting up the build is hard though 😞
Late to the train here :) But I'd +1 to OneAuth either exposing static linking via C ABI or an out-of-proc execution model. This would also benefit Rust programs as well for FFI (which seems to have increasing adoption).
How would you feel about shipping it in a beta?
I would be fine with this - just nervous about making it the default on windows right away, but happy to let folks opt into the world. I'd also be happy with us flighting it to say 20% of our windows users for the default interactive login mode and doing a progressive rollout that way. We have the technology now to do such things (and we did it for some earlier work) so happy to do that here as well.
@chlowell curious, what are the file sizes of these dependencies?
@chlowell curious, what are the file sizes of these dependencies?
Their release builds add up to ~4.3 MB.
I made a few changes to better support MSA scenarios:
--tenant-id
so users can specify the tenant for authentication--browser
) to bypass OneAuth at loginMay elevate using
sudo
on some platforms and configurations
bash:
curl -fsSL https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216/uninstall-azd.sh | bash;
curl -fsSL https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216/install-azd.sh | bash -s -- --base-url https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216 --version '' --verbose --skip-verify
pwsh:
Invoke-RestMethod 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216/uninstall-azd.ps1' -OutFile uninstall-azd.ps1; ./uninstall-azd.ps1
Invoke-RestMethod 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216/install-azd.ps1' -OutFile install-azd.ps1; ./install-azd.ps1 -BaseUrl 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216' -Version '' -SkipVerify -Verbose
PowerShell install
powershell -c "Set-ExecutionPolicy Bypass Process; irm 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216/uninstall-azd.ps1' > uninstall-azd.ps1; ./uninstall-azd.ps1;"
powershell -c "Set-ExecutionPolicy Bypass Process; irm 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216/install-azd.ps1' > install-azd.ps1; ./install-azd.ps1 -BaseUrl 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216' -Version '' -SkipVerify -Verbose;"
MSI install
powershell -c "irm 'https://azuresdkreleasepreview.blob.core.windows.net/azd/standalone/pr/3216/azd-windows-amd64.msi' -OutFile azd-windows-amd64.msi; msiexec /i azd-windows-amd64.msi /qn"
docker run -it azdevcliextacr.azurecr.io/azure-dev:pr-3216
This adds a build tag (
oneauth
) that replaces browser auth on Windows with OneAuth i.e., when this tag is set, azd will open OneAuth's login window instead of a browser. Other platforms and auth flows aren't affected. OneAuth is a C++ library, so calling into it from Go requires cgo and a C ABI. cgo requires a gcc compiler; I've used MSYS2 to install and configure MinGW. OneAuth doesn't have a C ABI, so this PR includes a bridge library that does. Building OneAuth for Windows (and hence the bridge library) requires MSVC. The go link tool can't link MSVC objects, so static linking is out of the question and distribution is awkward. I have azd embed the bridge DLL and write it to%LocalAppData%/azd
at runtime when needed.