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

LEAF-4653 - Power Bi Gateway #2679

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shaneodd
Copy link
Contributor

@shaneodd shaneodd commented Feb 10, 2025

Note on the branch name

I messed up the branch name, it should have been called Power Bi Gateway, this doesnt really have to do with large query besides none of this works even without the large query changes in place.

Summary

Currently if the user is in the national orgchart and you try and request data through power bi, power query or through a power bi gateway it will never get the data. In power bi and the gateway it will return with "We reached the end of the buffer" which would lead you to believe data would have issues and not authentication, however it is failing the authentication and returning an empty data set.

The fix

The issue is when the system is authenticating it is also requiring an email. A service account does not have an email address attached so the check for this causes it to fail. I have adjusted this to allow this user to log in even without the email address and adjusted the insertion of the email address to not include it if there is none. This appears to not have any defect from what I can find.

Research

I was trying to find how the old system would have authenticated. It appears everything for authentication appears to go down this same path. We do not have a historical reference of the auth servers code so I cannot be sure if this authenticated in any way. The code change here was added 6 years ago. Other and older files all appear to have email as part of this check. Only thing I can think of is this data was set previously for old service accounts and maybe an empty email address was in place previously which would explain why this check was probably fine in the past and now it does not work.

Impact

Do these service accounts get automatically pulled in or are we manually adding them? Some cases look manually added and others appear to be pulled in via the import scripts. There was some discussion between Jamie and I and this appears to be the case. There are some that are pulled in and some more that could possibly be pulled in. I would say this may be a separate discussion and could be expanded upon when we start getting people going on pulling data this way.

Testing

This is setup to allow accounts without email to be setup on the site for processes. When you run a process using a service account on a site that does not have that service account on you should now be able to run processes against.
image

@shaneodd shaneodd marked this pull request as ready for review March 6, 2025 18:13
@shaneodd
Copy link
Contributor Author

shaneodd commented Mar 6, 2025

Since this all redirects to the authserver I am not really able to write a test since it needs that hop. I also see that anytime I try to change this cookie it gets switched back to tester so more thought is needed than just this one test. I know there is additional work to allow for a http login which should help us move forward with this.

I tried making something to be a placeholder and it is starting to turn into too much for testing right now. I think we will need to do something to allow for auth testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants